lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] [lmi-commits] master 6730bd6 2/3: Add member irr_initialized_


From: Greg Chicares
Subject: Re: [lmi] [lmi-commits] master 6730bd6 2/3: Add member irr_initialized_ to keep track of state
Date: Fri, 23 Feb 2018 00:35:14 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2

On 2018-02-22 21:49, Vadim Zeitlin wrote:
> On Thu, 22 Feb 2018 20:25:30 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> In-class initialization would make it less obviously correct IMO.
> GC> When I reviewed the changeset one last time before committing it,
> GC> I used vim to step through all occurrences of
> GC>   /\<irr_initialized_\>
> GC> several times, to make sure they all looked right. Because it's
> GC> initialized in each ctor-initializer, I know that pressing 'n'
> GC> repeatedly, in the 'ledger_invariant.cpp' buffer only, shows me
> GC> every place where this member variable's value is set or used
> GC> throughout the class. Verifying correctness would be harder if
> GC> it could also acquire a value in the header.
> 
>  But what prevents an inline ctor from being defined in the header? And

Self-discipline. At least in this case.

> using ":Ggrep -w irr_initialized_" (or ":!git grep -w irr_initialized_"
> without vim-fugitive) is not that much harder than using "*".

  1
  *

  123456789012345678901234567890
  :!git grep -w irr_initialized_

It's thirty times as hard even if difficulty is only linear in keystroke count.

And it doesn't show everything in context--the tenth line is:
  ledger_invariant.cpp:    irr_initialized_ = true;
but, seeing it in isolation, I can't tell whether it'd correct.

> GC> I don't see in-class initialization as always preferable to
> GC> ctor-initializers. It may be better when we have many ctors, e.g.:
> GC> 
> GC>   http://www.stroustrup.com/C++11FAQ.html#member-init
> GC> | the real benefits come in classes with multiple constructors.
> GC> 
> GC> but here we have two ctors that repeat the same two initializers,
> GC> ten lines apart. Duplicating two lines is not a high price to pay
> GC> for keeping all initialization and assignment to this member
> GC> localized in a single file.
> 
>  The trouble is that it's inconsistent. An object will still be
> (default-)initialized even if you don't use any specific expression in its
> declaration, but a field of a primitive type, such as irr_initialized_,
> won't be. I don't see any reason for this difference and because we don't
> want to (nor can) keep the objects uninitialized, it seems logical to
> conclude that all fields should be initialized.

Inconsistency troubles me. I agree that _this_ particular member must
always be initialized.

But in this class I'm not sure _all_ members can readily be initialized.
I tried adding a compiler option to make sure of that, and...

  ‘LedgerInvariant::ErPmt’ should be initialized in the member initialization 
list [-Werror=effc++]
  [...snip myriad similar diagnostics...]

However, this is a weird class. I was struck by this, and wanted to comment on 
it
earlier in this thread:

LedgerInvariant& LedgerInvariant::operator=(LedgerInvariant const& obj)
{
    if(this != &obj)
        {
        LedgerBase::operator=(obj);
        Destroy();
        Alloc(obj.Length);
        Copy(obj);
        }
    return *this;
}

...but I had to wait until Ugolyok moved so that I could get Coplien's 1992 
book off
the shelf behind him, and...yes, that was my version of his "Orthodox Canonical 
Form".
Back then, the stack was so tiny that almost anything bigger than an int had to 
be a
pointer into the heap. At one time I wrote everything this way, but now only 
the ledger
classes preserve that old framework. Someday we should modernize this--it's 
costly to
maintain it, e.g., because I recently had to add a new member datum to 
Destroy(). (It
seemed weird to "destroy" a boolean, but I didn't want to start a major 
rewrite.)

>  I'd also like to say that I've never seen a bug in the code due to an
> extra initialization, while I did see (and wrote myself) bugs due to
> missing initialization more times than I can remember. So IMO initializing
> everything is much safer than not always doing it.

True. Okay, I'll add that, writing it in the "One True Way" according to:
  http://lists.nongnu.org/archive/html/lmi/2017-03/msg00024.html
In light of the discussion far above, and of the "orthodox canonical 
form"-ality of
this thing, I'm not willing to remove the ctor-initializers at this time: they 
still
govern, and '= {false};' in the header is just to make assurance doubly sure.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]