Christian Couder <christian.couder@xxxxxxxxx> 于2021年7月3日周六 上午6:11写道：
>
> On Fri, Jul 2, 2021 at 9:28 PM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> >
> > Aside from the potential maintenance burden of the `atom_type >= ...
> > && atom_type <= ...` approach, another problem is that it increases
> > cognitive load. The reader has to go reference the enum to fully
> > understand the cases to which this code applies. On the other hand,
> > the way the patch mentions the enumeration items explicitly, it is
> > obvious at-a-glance to which cases the code applies. An additional
> > downside of the suggestion is that the range specified by `>=` and
> > `<=` may cause some readers to think that there is some sort of
> > implicit relationship between the items in the range, which doesn't
> > seem to be the case. So, I find the way it's done in the patch
> > presently easier to comprehend.
>
> I agree that it's less cognitive load, but maybe it could be improved
> using a separate function like:
>
> static int reject_atom(int cat_file_mode, enum atom_type atom_type)
> {
> if (!cat_file_mode)
> return atom_type == ATOM_REST;
>
> /* cat_file_mode */
> switch (atom_type) {
> case ATOM_FLAG:
> case ATOM_HEAD:
> case ATOM_PUSH:
> case ATOM_REFNAME:
> case ATOM_SYMREF:
> case ATOM_UPSTREAM:
> case ATOM_WORKTREEPATH:
> return 1;
> default:
> return 0;
> }
> }
I agree. It is clearer to use a function to wrap origin reject atoms step.
Thanks.
--
ZheNIng Hu
