Re: [lmi] [lmi-commits] master 9b3d0f1: Write explicitly-defaulted [cd]tors inline, in class defn

[Vadim Zeitlin]

On Sun, 5 Mar 2017 20:43:31 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2017-03-05 19:54, Vadim Zeitlin wrote:
GC> > On Sun,  5 Mar 2017 14:28:23 -0500 (EST) Greg Chicares <address@hidden> 
wrote:
GC> [...heavily trimmed...]
GC> > GC> diff --git a/authenticity.hpp b/authenticity.hpp
GC> > GC> index 447b8f6..0ae15ce 100644
GC> > GC> --- a/authenticity.hpp
GC> > GC> +++ b/authenticity.hpp
GC> [...]
GC> > GC> -    mutable calendar_date CachedDate_;
GC> > GC> +    mutable calendar_date CachedDate_ = calendar_date(jdn_t(0));
GC> > 
GC> >  Looking at this, I can't help returning to our discussion about using "="
GC> > vs "()" vs "{}" because this type name repetition seems unfortunate and 
I'd
GC> > clearly prefer to apply the following change here:
GC> [...more heavy trimming...]
GC> > -    mutable calendar_date CachedDate_ = calendar_date(jdn_t(0));
GC> > +    mutable calendar_date CachedDate_{jdn_t(0)};
GC> > 
GC> > Wouldn't you?
GC> 
GC> Yes, of course. It didn't even compile the first time because I wrote
GC>     mutable calendar_date CachedDate_ = jdn_t(0);
GC> which was okay in the (replaced) ctor-initializer, but not here.
GC> 
GC> Uniform initialization here would be like casting to auto: it tells
GC> the compiler to do what it already knows how to do better than us.

 Exactly.

GC> > GC> -    double ScalarIMF_;
GC> > GC> -    std::string ShortName_;
GC> > GC> -    std::string LongName_;
GC> > GC> -    std::string gloss_;
GC> > GC> +    double      ScalarIMF_ = 0.0;
GC> > GC> +    std::string ShortName_ = std::string();
GC> > GC> +    std::string LongName_  = std::string();
GC> > GC> +    std::string gloss_     = std::string();
GC> > GC>  };
GC> > 
GC> >  The last 3 new lines above also seem unnecessarily verbose to me, IMHO
GC> > this is what default ctors are for and we could just omit the
GC> > initialization entirely, but if we wanted to keep it for explicitness 
sake,
GC> > I'd use "std::string ShortName_{};" etc here.
GC> 
GC> In this and the other examples you gave, I thought I was following
GC> the rule we agreed on, e.g., here:
GC> 
GC> void input_sequence_test::check
GC>     (char const*                     file
GC> ...
GC>     ,char const*                     m = ""
GC>     ,std::vector<std::string> const& k = std::vector<std::string>()
GC>     ,std::vector<std::string> const& c = std::vector<std::string>()
GC>     ,bool                            o = false
GC>     ,std::string const&              w = std::string()
GC> 
GC> But even if I remember that correctly, there's no reason why our
GC> thinking shouldn't evolve.

 I thought the rule applied to the function arguments, but I didn't realize
it would be applied to the in-class member initialization too. I really
need to at least create the style guidelines document I keep speaking about
if only in order to add the new rules to it.

 Anyhow, in the light of these examples, what do you now think the rule
should be for the member initialization? Personally, I like using the
simple "member_ = xxx" for primitive types such as int or bool, because it
seems more readable to me, as the values are clearly separated from the
fields, especially with the vertical alignment convention used in lmi, but
I also like to use "member_{xxx}" for the non-primitive types to avoid
repeating the type name, especially in the special case of "member_{}", if
we decide to explicitly default-initialize all members. The latter seems to
outweigh the former, so I think we should just standardize on using uniform
initialization here (and also in the member initialization lists because
they are very similar).

 What do you think?
VZ