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

From: Junio C Hamano <[email-masked]>
Date: Thu, 31 Oct 2013 13:31:58 -0700

Martin von Zweigbergk <[email-masked]> writes:

> 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.

That follows "visual/textual order should follow the actual
ordering" principle.  Think of a number-line you learn in elementary
school arithmetic class, and try to place revs.nr and i on it.

I agree that there is no justification to write "if 0 == something",
when "if something == 0" suffices.  The latter reads better and that
is why the phrase "yoda condition" was invented.

But the situation is different when both sides are not constants,
and especially when "<" and "<=" are involved..