Re: [PATCH v2 2/2] [GSOC] ref-filter: introduce enum atom_type

2 messages, 2 authors, 2021-05-11 · open the first message on its own page

Re: [PATCH v2 2/2] [GSOC] ref-filter: introduce enum atom_type

From: Junio C Hamano <hidden>
Date: 2021-05-11 06:12:10

Christian Couder [off-list ref] writes:
quoted
I find it far more intuitive to say

        for (i = 0; i < ATOM_INVALID; i++)

than having to say UNKNOWN+1.
Yeah, that's more intuitive. But in my opinion, using `ATOM_UNKNOWN +
1` instead of `0` at least shouldn't often result in more lines of
code, and should be a bit easier to get right, compared to having to
initialize the field with ATOM_UNKNOWN.
Number of lines is not all that important.

But the developers must remember that UNKNOWN is at the bottom end
and INVALID is at the top end, which is very taxing.  Tying UNKNOWN
to the top end and INVALID to the bottom end would equally be
plausible and there is no memory aid to help us remember which one
is which.  Compare it to "array indices begin at 0, and the upper
end is MAX".  Your scheme is much easier for developers to screw up.

Re: [PATCH v2 2/2] [GSOC] ref-filter: introduce enum atom_type

From: ZheNing Hu <hidden>
Date: 2021-05-11 12:53:31

Junio C Hamano [off-list ref] 于2021年5月11日周二 下午2:12写道:
Christian Couder [off-list ref] writes:
quoted
quoted
I find it far more intuitive to say

        for (i = 0; i < ATOM_INVALID; i++)

than having to say UNKNOWN+1.
Yeah, that's more intuitive. But in my opinion, using `ATOM_UNKNOWN +
1` instead of `0` at least shouldn't often result in more lines of
code, and should be a bit easier to get right, compared to having to
initialize the field with ATOM_UNKNOWN.
Number of lines is not all that important.

But the developers must remember that UNKNOWN is at the bottom end
and INVALID is at the top end, which is very taxing.  Tying UNKNOWN
to the top end and INVALID to the bottom end would equally be
plausible and there is no memory aid to help us remember which one
is which.  Compare it to "array indices begin at 0, and the upper
end is MAX".  Your scheme is much easier for developers to screw up.
Yes, UNKNOWN + 1 is difficult to use. But using UNKNOWN = -1,
this means that the coder may indirectly use an init atom_type with
junk value "ATOM_REFNAME", they maybe did't notice they need
reinitialize the value to UNKNOWN.

I thought that perhaps such a naming would be better:

ATOM_BEGIN = ATOM_UNKNOWN + 1,
ATOM_END = ATOM_INVALID

       for (i = ATOM_BEGIN; i < ATOM_END; i++) {
       }

But ATOM_END has been used...

--
ZheNing Hu
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help