dbwrap_tool: Simplify persistent/non-persistent check / dbwrap_tool: Avoid an unnecessary "else"

[Ralph Böhme]

Moin,

On Tue, Sep 04, 2018 at 09:43:32AM +1200, Andrew Bartlett wrote:
>On Mon, 2018-09-03 at 23:30 +0200, Ralph Böhme wrote:
>> Howdy,
>>
>> On Tue, Sep 04, 2018 at 09:11:32AM +1200, Andrew Bartlett wrote:
>> > While each of these looks reasonable on its own, is this really the
>> > clearest (rather than cleverest) possible way this could have been
>> > constructed as a whole?
>>
>> s/clearest/clearer/
>>
>> yes. The clearest way? Who knows. Patches welcome.
>>

1)

>> > > -       if ((persistent == 0 && non_persistent == 0) ||
>> > > -           (persistent == 1 && non_persistent == 1))
>> > > -       {
>> > > +       if ((persistent + non_persistent) != 1) {
>> > >                 d_fprintf(stderr, "ERROR: you must specify exactly one "
>> > >                           "of --persistent and --non-persistent\n");
>> > >                 goto done;

2)

>> > > -       } else if (non_persistent == 1) {
>> > > +       }
>> > > +       if (non_persistent == 1) {
>> > >                 tdb_flags |= TDB_CLEAR_IF_FIRST;
>> > >         }
>> >
>> > What specifically was the goal of the change here?
>>
>> What specifically is the goal of this rhetorical question?
>
>I was hoping to hear a rationale as to how this is 'simplified',

hm, to me this seems obvious.

1) The old expression was more complex then the new one:

old: ((expr && expr) || (expr && expr))
new: (expr)

Imho this is not a matter of taste and style. Even while I might not nack such 
complex expressions while reviewing code (eventually I would), I'm happy to ack 
patches that simplifies them.

2) Because the new code makes the logic much more clear and I would nack the old 
code in a review if this was in a patchset pending review.

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG Key Fingerprint:           FAE2 C608 8A24 2520 51C5
                               59E4 AA1E 9B71 2639 9E46