public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Martin von Zweigbergk <martinvonz@gmail•com>
Cc: Felipe Contreras <felipe.contreras@gmail•com>, git <git@vger•kernel.org>
Subject: Re: [PATCH 16/16] add: avoid yoda conditions
Date: Thu, 31 Oct 2013 13:31:58 -0700	[thread overview]
Message-ID: <xmqqmwlp41xd.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CANiSa6i5z8Z9HPzsUWTh8U2HXc9p6MPgQjJ7K6KSDw8FXtFyww@mail.gmail.com> (Martin von Zweigbergk's message of "Thu, 31 Oct 2013 12:48:57 -0700")

Martin von Zweigbergk <martinvonz@gmail•com> 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..

  parent reply	other threads:[~2013-10-31 20:32 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-31  9:25 [PATCH 00/16] Trivial patches Felipe Contreras
2013-10-31  9:25 ` [PATCH 01/16] merge: simplify ff-only option Felipe Contreras
2013-10-31 18:04   ` Junio C Hamano
2013-10-31  9:25 ` [PATCH 02/16] t: replace pulls with merges Felipe Contreras
2013-10-31  9:25 ` [PATCH 03/16] pull: cleanup documentation Felipe Contreras
2013-10-31 18:11   ` Junio C Hamano
2013-10-31 18:37     ` Felipe Contreras
2013-10-31 19:00       ` Junio C Hamano
2013-10-31 19:27         ` Max Horn
2013-10-31 19:42           ` Junio C Hamano
2013-10-31 20:43             ` Junio C Hamano
2013-10-31 21:16               ` Felipe Contreras
2013-10-31 23:40                 ` David Aguilar
2013-11-01  1:56                   ` Felipe Contreras
2013-11-01  2:48                     ` David Aguilar
2013-11-01  3:50                       ` Felipe Contreras
2013-11-01 11:05                         ` SZEDER Gábor
2013-11-01 12:20                           ` Felipe Contreras
2013-10-31 19:51         ` Felipe Contreras
2013-10-31 20:27           ` Junio C Hamano
2013-10-31 21:15             ` Felipe Contreras
2013-10-31  9:25 ` [PATCH 04/16] fetch: add missing documentation Felipe Contreras
2013-10-31 18:10   ` Junio C Hamano
2013-10-31 19:08     ` Felipe Contreras
2013-10-31  9:25 ` [PATCH 05/16] revision: add missing include Felipe Contreras
2013-10-31 18:11   ` Junio C Hamano
2013-10-31  9:25 ` [PATCH 06/16] shortlog: add missing declaration Felipe Contreras
2013-10-31 19:05   ` Junio C Hamano
2013-10-31 19:33     ` Felipe Contreras
2013-10-31 20:07       ` Junio C Hamano
2013-10-31 21:17         ` Felipe Contreras
2013-10-31  9:25 ` [PATCH 07/16] branch: trivial style fix Felipe Contreras
2013-10-31 20:46   ` Junio C Hamano
2013-10-31  9:25 ` [PATCH 08/16] sha1-name: trivial style cleanup Felipe Contreras
2013-10-31  9:25 ` [PATCH 09/16] transport-helper: trivial style fix Felipe Contreras
2013-10-31  9:25 ` [PATCH 10/16] describe: trivial style fixes Felipe Contreras
2013-10-31  9:25 ` [PATCH 11/16] pretty: trivial style fix Felipe Contreras
2013-10-31  9:25 ` [PATCH 12/16] revision: trivial style fixes Felipe Contreras
2013-10-31  9:25 ` [PATCH 13/16] diff: trivial style fix Felipe Contreras
2013-10-31  9:25 ` [PATCH 14/16] run-command: trivial style fixes Felipe Contreras
2013-10-31  9:25 ` [PATCH 15/16] setup: " Felipe Contreras
2013-10-31 20:48   ` Junio C Hamano
2013-10-31  9:25 ` [PATCH 16/16] add: avoid yoda conditions Felipe Contreras
2013-10-31 19:48   ` Martin von Zweigbergk
2013-10-31 19:56     ` Felipe Contreras
2013-10-31 20:31     ` Junio C Hamano [this message]
2013-10-31 20:42       ` Felipe Contreras
2013-10-31 18:03 ` [PATCH 00/16] Trivial patches Max Horn

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=xmqqmwlp41xd.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=felipe.contreras@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=martinvonz@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox