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

From: ZheNing Hu <adlternative@gmail.com>
Date: May 11, 2021, 12:37 UTC
Message-ID: <CAOLTT8RP9B7mmP+zfgdjrP+jjoBgJyeWmxnJitOFM87JraXCFQ@mail.gmail.com>
In-Reply-To: <CAP8UFD3m0TA3nPDaPhaU3=FVF_r5cPM8xksVL6MpCOhNC4dT3g@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> 于2021年5月11日周二 下午1:51写道：

Show 8 quoted lines

>
> >
> > Shouldn't it be (-1)?
>
> If it's -1 instead of 0, then it might be a bit more complex to
> initialize structs that contain such a field, as it cannot be done
> with only xcalloc().
>

I agree. If the traverse start from 0, an init atom_type will have "ATOM_REFNAME" junk value. If let users manually adjust it to ATOM_UNKNOWN, it seems to be very troublesome.

Show 18 quoted lines

> > And I'd assume I am right in the following.
> >
> > > + * ATOM_INVALID equals to the size of valid_atom array, which could help us
> > > + * iterate over valid_atom array like this:
> > > + *
> > > + *   for (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++) {
> >
> > 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.
>

There will be a trade-off. Traverse from 0 or does not need to adjust the initialized atom_type = UNKNOWN.

Show 10 quoted lines

>
> > It would allow people to say
> >
> >         for (i = 0; i < ATOM_MAX; i++)
> >
> > instead, which would be even nicer.
>
> Yeah, it's also a tradeoff to have the last one called ATOM_MAX
> instead of ATOM_INVALID, and to have a separate ATOM_INVALID if it's
> needed.

About ATOM_MAX or ATOM_INVALID, I have a idea:

enum atom_type {
ATOM_UNKNOWN,
...
ATOM_ELSE,
ATOM_INVALID,
+ATOM_MAX = ATOM_INVALID
 };

This might be able to do both.

Thanks. -- ZheNing Hu