Subject: [bug#58812] Coding style: similarly-named variables

From: Ludovic Courtès <[email-masked]>
Date: Thu, 17 Nov 2022 18:37:39 +0100

Hi,

Maxim Cournoyer <[email-masked]> skribis:

> Ludovic Courtès <[email-masked]> writes:
>
>> Maxim Cournoyer <[email-masked]> skribis:
>>
>>> * gnu/build/install.scm (evaluate-populate-directive): By default, error when
>>> the target of a symlink doesn't exist.  Always ensure TARGET ends with "/".
>>> (populate-root-file-system): Call evaluate-populate-directive with
>>>  #:error-on-dangling-symlink #t and add comment.
>>
>> [...]
>>
>>> +  (define target* (if (string-suffix? "/" target)
>>> +                      target
>>> +                      (string-append target "/")))
>>
>> Maybe make it:
>>
>>   (let ((target (if …)))
>>     …)
>>
>> so there’s only one ‘target’ in scope (and no ‘target*’); otherwise it’s
>> easy to forget the ‘*’ and refer to wrong one.
>
> It's a pattern I've used at other places; I find it more hygienic to not
> shadow existing variables; it signal to the reader "be careful, this is
> not the same as the argument-bound one, though they are closely
> related".

I don’t buy it.  :-)  The reader might be careful yet end up using the
“wrong” variable.  As long as the “wrong” variable has no use, I think
it’s best to shadow it so that mistakes cannot happen.

Of course the details vary depending on context, but I think we should
not start introducing this pattern in different places.  Perhaps
something to discuss and codify under “Formatting Code”?

Ludo’.