[neomutt-devel] Uniformize structures storing operations

[Guyzmo]

On Wed, Feb 15, 2017 at 11:41:49AM -0500, Damien Riegel wrote:
> 1. a) Omitted field members
> 
> Side question: do we prefer implicit or explicit member initialization?

if there's a guarantee that the unasigned values have a NULL (or a
common know value at compile time), I'm ok with that. When a method is
not implemented we need to fail soon, fail hard, but fail gracefully.
In order to do that, we must be able to find there's a missing method.

> 2. Typedef'ing function pointers
> 
> […] Moreover in the coding style discussion, it seems
> we generally want to avoid typedefs (except in a few cases). So does it
> also apply here?

Generally speaking function pointer types are a pain to read and even
more to write. So I believe that it's fair to use typedefs when the
following conditions are met:

① you're likely to manipulate the FPs using intermediate variables (cf
  your stackoverflow example where you loop over the signals), or
  functions (like: SignalHanlder *get_hanlder(int);),
② you're likely to reference the FPs in several places far away from the
  current compilation unit, which means the FP is a thing to be
  implemented by an user and fed as argument to your (documented) API.

To implement the strategy pattern as you suggest, we're having the user
of the API declare a function matching the prototype and assign it to a
struct. So at no time he'll actually see the FP type. Then in places
where they will be used, the FP will be used using the member of the
instanciated struct, meaning that you won't neither see the type there.

So my opinion is that in the current case: it's not needed.

> 3. Using macros for declaration
> 
> In hcache, a macro is used to declare the different kinds of hcache
> backend. It prevents some boilerplate code but it forces function naming
> and "obfuscates" some info. So for instance, if I want to check in what
> context `hcache_gdbm_open` is used it is harder to track because there
> is only one occurrence of this symbol: its definition, but no explicit
> affectation nor call. Of course, it's easy to understand what's going on
> once you saw the macro, but before that it's more confusing.

To be honest, I wouldn't mind getting rid of that macro, because as you
say it would make things a bit more explicit. Or, as an alternative, add
some documentation at the top of the source telling the reader that all
the methods follow a strict naming scheme used by the macro XX_OPS to
build the struct xx_ops_t.

What I think is really important is to stay homogeneous, whichever we
choose, it has to be the same for all the others.

> […] That way, understanding one part of the codebase helps
> understanding other parts.

Exactly, as I said: ho-mo-ge-ne-i-ty!

-- 
zmo