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

From: ZheNing Hu <adlternative@gmail.com>
Date: May 11, 2021, 12:53 UTC
Message-ID: <CAOLTT8T-pUc7SOaivdydXHXm+_-hY=ARpWqGA_OOnej8gRrMCA@mail.gmail.com>
In-Reply-To: <xmqq7dk5ol3c.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> 于2021年5月11日周二 下午2:12写道：

Show 23 quoted lines

>
> Christian Couder <christian.couder@gmail.com> writes:
>
> >> 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