[PATCH 4/6] cli: intialize crypto structure in show and reply

Jani Nikula jani at nikula.org
Thu May 17 13:23:15 PDT 2012


On Thu, 17 May 2012, Jameson Graef Rollins <jrollins at finestructure.net> wrote:
> On Thu, May 17 2012, Jani Nikula <jani at nikula.org> wrote:
>> The values are not undefined, they are properly initialized, and we can
>> count on it. For sure, not maybe. If you want to explicitly set them for
>> clarity, it's a matter of taste. Personally I find it too verbose, but then
>> again notmuch code is generally fairly verbose.
>
> I want them explicitly set for clarity, as well as safety.  Code is
> meant to be read by humans, not computers.  Brevity is not always a
> virtue if it sacrifices clarity.  It's much nicer to have the defaults
> clearly stated in the initialization, than to force the reader to
> understand how the initialization works and to interpret what that means
> for the current case.  I also don't think it's safe to assume that the
> variables will be always be "properly" initialized in your favor in
> perpetuity.  It's much safer to explicitly set them to what you want
> them to be rather than just assume they'll be set correctly.

In short, when you read enough code, having everything explicitly stated
becomes a burden. It's explicit, but you have to read it all, even when
there really is no need to.

>> If you insist on it, please at least drop the extra temp crypto
>> variable, and initialize the struct in one initializer.
>
> I don't see why this matters either.  Again, I think this is just a
> matter of taste.  I would rather the code be verbose where clarity
> requires it, rather than always trying to make the code as terse as
> possible.

You introduce an extra variable that every reader of your code has to
track down to realize that it's only ever used once to initialize
another variable. You make code harder for other people to read.

I have now offered my review and opinions on the matter; I will not
pursue this discussion further.


BR,
Jani.


More information about the notmuch mailing list