From: ZheNing Hu <adlternative@gmail.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
ZheNing Hu via GitGitGadget <gitgitgadget@gmail.com>,
git <git@vger.kernel.org>, Jeff King <peff@peff.net>,
Hariom Verma <hariom18599@gmail.com>
Subject: Re: [PATCH v2 2/2] [GSOC] ref-filter: introduce enum atom_type
Date: Tue, 11 May 2021 20:37:33 +0800 [thread overview]
Message-ID: <CAOLTT8RP9B7mmP+zfgdjrP+jjoBgJyeWmxnJitOFM87JraXCFQ@mail.gmail.com> (raw)
In-Reply-To: <CAP8UFD3m0TA3nPDaPhaU3=FVF_r5cPM8xksVL6MpCOhNC4dT3g@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> 于2021年5月11日周二 下午1:51写道：
>
> >
> > 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.
> > 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.
>
> > 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
next prev parent reply other threads:[~2021-05-11 12:38 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-08 15:22 [PATCH 0/2] [GSOC][RFC] ref-filter: introduce enum atom_type ZheNing Hu via GitGitGadget
2021-05-08 15:22 ` [PATCH 1/2] [GSOC] ref-filter: add objectsize to used_atom ZheNing Hu via GitGitGadget
2021-05-08 15:22 ` [PATCH 2/2] [GSOC][RFC] ref-filter: introduce enum atom_type ZheNing Hu via GitGitGadget
2021-05-09 6:21 ` Christian Couder
2021-05-09 8:26 ` Junio C Hamano
2021-05-09 13:44 ` ZheNing Hu
2021-05-09 13:40 ` ZheNing Hu
2021-05-10 15:03 ` [PATCH v2 0/2] " ZheNing Hu via GitGitGadget
2021-05-10 15:03 ` [PATCH v2 1/2] [GSOC] ref-filter: add objectsize to used_atom ZheNing Hu via GitGitGadget
2021-05-10 15:03 ` [PATCH v2 2/2] [GSOC] ref-filter: introduce enum atom_type ZheNing Hu via GitGitGadget
2021-05-11 2:14 ` Junio C Hamano
2021-05-11 5:51 ` Christian Couder
2021-05-11 6:12 ` Junio C Hamano
2021-05-11 12:53 ` ZheNing Hu
2021-05-11 12:37 ` ZheNing Hu [this message]
2021-05-11 13:05 ` Junio C Hamano
2021-05-11 12:18 ` ZheNing Hu
2021-05-12 12:11 ` [PATCH v3 0/2] [GSOC][RFC] " ZheNing Hu via GitGitGadget
2021-05-12 12:11 ` [PATCH v3 1/2] [GSOC] ref-filter: add objectsize to used_atom ZheNing Hu via GitGitGadget
2021-05-12 23:11 ` Junio C Hamano
2021-05-13 9:04 ` ZheNing Hu
2021-05-12 12:11 ` [PATCH v3 2/2] [GSOC] ref-filter: introduce enum atom_type ZheNing Hu via GitGitGadget
2021-05-12 23:21 ` Junio C Hamano
2021-05-13 9:25 ` ZheNing Hu
2021-05-13 15:15 ` [PATCH v4 0/2] [GSOC][RFC] " ZheNing Hu via GitGitGadget
2021-05-13 15:15 ` [PATCH v4 1/2] [GSOC] ref-filter: add objectsize to used_atom ZheNing Hu via GitGitGadget
2021-05-13 15:15 ` [PATCH v4 2/2] [GSOC] ref-filter: introduce enum atom_type ZheNing Hu via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAOLTT8RP9B7mmP+zfgdjrP+jjoBgJyeWmxnJitOFM87JraXCFQ@mail.gmail.com \
--to=adlternative@gmail.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=hariom18599@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.