[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#58812] Coding style: similarly-named variables
From: |
Maxim Cournoyer |
Subject: |
[bug#58812] Coding style: similarly-named variables |
Date: |
Thu, 17 Nov 2022 15:34:27 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Hi,
Ludovic Courtès <ludo@gnu.org> writes:
> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>> Maxim Cournoyer <maxim.cournoyer@gmail.com> 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.
I'm surprised you're not buying it, given we're writing Scheme in a more
functional style, and mutating same-named variables clearly goes against
that style :-).
> 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”?
That's more of a coding style guidelines than "formatting" code (when I
read "formatting", I think of a mechanical process like 'guix style' or
'rust-fmt' can do), but yes, that could be nice to have. Better yet,
something basic to share across the whole Guile/Scheme community and
include in the Guile user manual, like Python has PEP 8 they can refer
to, to save every Guile/Scheme project from having to reinvent the
wheel.
--
Thanks,
Maxim
- [bug#58812] [PATCH 0/5] Add --symlink option to 'guix shell'., (continued)
- [bug#58812] [PATCH 0/5] Add --symlink option to 'guix shell'., Ludovic Courtès, 2022/11/14
- [bug#58812] [PATCH v3 1/4] Makefile.am: Sort EXTRA_DIST entries., Maxim Cournoyer, 2022/11/10
- [bug#58812] [PATCH v3 2/4] install: Validate symlink target in evaluate-populate-directive., Maxim Cournoyer, 2022/11/10
- [bug#58812] [PATCH v3 4/4] shell: Detect --symlink spec problems early., Maxim Cournoyer, 2022/11/10
- [bug#58812] [PATCH v3 3/4] guix: shell: Add '--symlink' option., Maxim Cournoyer, 2022/11/10
[bug#58812] [PATCH 0/5] Add --symlink option to 'guix shell'., zimoun, 2022/11/16
[bug#58812] [PATCH 0/5] Add --symlink option to 'guix shell'., Ludovic Courtès, 2022/11/09
- [bug#58812] [PATCH 0/5] Add --symlink option to 'guix shell'., Maxim Cournoyer, 2022/11/09
- [bug#58812] [bug#59164] Coding style: similarly-named variables, zimoun, 2022/11/17
- [bug#58812] [bug#59164] Coding style: similarly-named variables, Maxim Cournoyer, 2022/11/18
- [bug#58812] [bug#59164] Coding style: similarly-named variables, zimoun, 2022/11/21
- [bug#58812] [bug#59164] Coding style: similarly-named variables, zimoun, 2022/11/21
- [bug#59164] [bug#58812] [bug#59164] Coding style: similarly-named variables, Maxim Cournoyer, 2022/11/21
- [bug#58812] [bug#59164] Coding style: similarly-named variables, zimoun, 2022/11/22
- [bug#58812] [bug#59164] Coding style: similarly-named variables, Ludovic Courtès, 2022/11/26
[bug#58812] [PATCH 0/5] Add --symlink option to 'guix shell'., Ludovic Courtès, 2022/11/09