Re: [lmi] [lmi-commits] master 6730bd6 2/3: Add member irr_initialized_ to keep track of state

[Greg Chicares]

On 2018-02-17 12:14, Vadim Zeitlin wrote:
> On Fri, 16 Feb 2018 17:39:03 -0500 (EST) Greg Chicares <address@hidden> wrote:
> 
> GC> branch: master
> GC> commit 6730bd6dd2c986297f7cbee10471e9006c985bc3
> GC> Author: Gregory W. Chicares <address@hidden>
> GC> Commit: Gregory W. Chicares <address@hidden>
> GC> 
> GC>     Add member irr_initialized_ to keep track of state
> GC>     
> GC>     The calculation summary invokes CalculateIrrs() only when necessary,
> GC>     because IRR calculations affect responsiveness. The technique it uses
> GC>     to infer whether IRR calculations have already been run is extravagant
> GC>     and imperfect. This newly-added member offers an alternative that is
> GC>     simple, direct, and easier to perfect.
> [...]
> GC> diff --git a/ledger_invariant.hpp b/ledger_invariant.hpp
> GC> index c2f4d38..628a5d0 100644
> GC> --- a/ledger_invariant.hpp
> GC> +++ b/ledger_invariant.hpp
> [...]
> GC> @@ -414,9 +415,15 @@ class LMI_SO LedgerInvariant
> GC>      // Special cases.
> GC>      int             Length;
> GC>      int             irr_precision_;
> GC> -    bool            FullyInitialized;   // I.e. by Init(BasicValues 
> const* b).
> GC> +    bool            irr_initialized_;  // CalculateIrrs() succeeded
> GC> +    bool            FullyInitialized;  // Init(BasicValues const*) 
> succeeded
> GC>  };
> 
>  This is hardly very import, but I just wonder if we shouldn't use
> initializers for the new members even when this would introduce (minor)
> inconsistency with the existing code. I.e. in this case, I'd have written
> "bool irr_initialized_ = false;" instead of adding it to the ctor because
> it would make the changes more local and more obviously correct -- and
> would resolve the resulting inconsistency later by initializing
> "FullyInitialized" when declaring it too, rather than doing it in the ctor
> as well.

In-class initialization would make it less obviously correct IMO.
When I reviewed the changeset one last time before committing it,
I used vim to step through all occurrences of
  /\<irr_initialized_\>
several times, to make sure they all looked right. Because it's
initialized in each ctor-initializer, I know that pressing 'n'
repeatedly, in the 'ledger_invariant.cpp' buffer only, shows me
every place where this member variable's value is set or used
throughout the class. Verifying correctness would be harder if
it could also acquire a value in the header.

I don't see in-class initialization as always preferable to
ctor-initializers. It may be better when we have many ctors, e.g.:

  http://www.stroustrup.com/C++11FAQ.html#member-init
| the real benefits come in classes with multiple constructors.

but here we have two ctors that repeat the same two initializers,
ten lines apart. Duplicating two lines is not a high price to pay
for keeping all initialization and assignment to this member
localized in a single file.