Re: [PATCH 16/16] add: avoid yoda conditions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I was recently confused by the yoda condition in this block of code from [1]

+ for (i = 0; i < revs.nr; i++)
+ if (&bases->item->object == &revs.commit[i]->object)
+ break; /* found */
+ if (revs.nr <= i)

I think I was particularly surprised because it came so soon after the
"i < revs.nr". I didn't bother commenting because it seemed too
subjective and the code base has tons of these. Something as simple as

  git grep '[0-9] [<>]' *.c

finds a bunch (probably with lots of false positives and negatives).

I guess what I'm trying to say is that either we accept them and get
used to reading them without being surprised, or we can change a bit
more than one at a time perhaps? I understand that this was an
occurrence you just happened to run into, and I'm not saying that a
patch has to deal with _all_ occurrences. I'm more just wondering if
we want mention our position, whatever it is, in CodingGuidelines.

Martin

[1] http://thread.gmane.org/gmane.comp.version-control.git/236252/focus=236716

On Thu, Oct 31, 2013 at 2:25 AM, Felipe Contreras
<felipe.contreras@xxxxxxxxx> wrote:
> Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
> ---
>  builtin/add.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index 226f758..9b30356 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -429,7 +429,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>         argc--;
>         argv++;
>
> -       if (0 <= addremove_explicit)
> +       if (addremove_explicit >= 0)
>                 addremove = addremove_explicit;
>         else if (take_worktree_changes && ADDREMOVE_DEFAULT)
>                 addremove = 0; /* "-u" was given but not "-A" */
> --
> 1.8.4.2+fc1
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]