dbwrap_tool: Simplify persistent/non-persistent check / dbwrap_tool: Avoid an unnecessary "else"
Volker Lendecke
Volker.Lendecke at SerNet.DE
Tue Sep 4 03:33:39 UTC 2018
On Tue, Sep 04, 2018 at 09:11:32AM +1200, Andrew Bartlett via samba-technical 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?
As others said: This is much easier *for me* to read. I don't really
depend on this patch. Bikeshedding, but can you propose something
simpler?
> > - 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;
> > - } else if (non_persistent == 1) {
> > + }
> > + if (non_persistent == 1) {
> > tdb_flags |= TDB_CLEAR_IF_FIRST;
> > }
>
> What specifically was the goal of the change here?
Whenever I see an "else" with a "return" right before, the "else" is
redundant. The "else" makes me go back to the "if-condition" and see
what it was. Without the "else" this special condition is just done
and out of my mind. Easier flow to read for me.
Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
Meet us at Storage Developer Conference (SDC)
Santa Clara, CA USA, September 24th-27th 2018
More information about the samba-technical
mailing list