[Freeipa-devel] [PATCH] detect failure to write ipa_kpasswd.pid file

[Jim Meyering]

Simo Sorce <ssorce at redhat.com> wrote:
...
>> I have the opposite preference.
>> I prefer to avoid the added line (more code fits on a screen/page),
>> and to avoid the duplicate use of the variable name (one fewer detail
>> that can get out of sync).
>
> I hate it when debugging like I hate things like:
> if ((xyz = abc(def(foo))) == bar) { ...
> it makes debugging unnecessarily harder.

When xyz is a short name, sometimes I too prefer to avoid the
assignment-in-condition.  Performing the assignment as a separate
statement makes it obvious that there is an actual assignment:

    xyz = abc(def(foo));
    if (xyz == bar) { ...

However, when the assignment LHS is long enough to require
significant visual work to confirm that it is the same one
being tested on the next line, then I much prefer to avoid
the duplication.  I.e., I find this hard to read/maintain:

    kset->keys[i].ekey->value.bv_val = malloc(len+2);
    if (!kset->keys[i].ekey->value.bv_val) { ...

and prefer the code where I don't have to visually match long
strings (too easy to miss details like s/i/j/ or s/ekey/akey/):

    if ((kset->keys[i].ekey->value.bv_val = malloc(len+2)) != NULL) { ...

I.e., obscuring the assignment hinders readability,
but it's worth doing in cases like this.

> But this is a matter of personal choice I guess.

It's a balancing act.