lmi
[Top][All Lists]
Advanced

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

Re: [lmi] [lmi-commits] master 9b3d0f1: Write explicitly-defaulted [cd]t


From: Vadim Zeitlin
Subject: Re: [lmi] [lmi-commits] master 9b3d0f1: Write explicitly-defaulted [cd]tors inline, in class defn
Date: Sun, 5 Mar 2017 20:54:22 +0100

On Sun,  5 Mar 2017 14:28:23 -0500 (EST) Greg Chicares <address@hidden> wrote:

GC> diff --git a/authenticity.hpp b/authenticity.hpp
GC> index 447b8f6..0ae15ce 100644
GC> --- a/authenticity.hpp
GC> +++ b/authenticity.hpp
GC> @@ -43,8 +43,8 @@ enum {md5len = 128 / CHAR_BIT};
GC>  /// Implemented as a simple Meyers singleton, with the expected
GC>  /// dead-reference and threading issues.
GC>  ///
GC> -/// 'cached_date_' holds the most-recently-validated date, or a
GC> -/// peremptorily-invalid default value of JDN zero.
GC> +/// 'cached_date_' holds the most-recently-validated date; it is
GC> +/// initialized to a peremptorily-invalid default value of JDN zero.
GC>  
GC>  class Authenticity final
GC>  {
GC> @@ -58,14 +58,14 @@ class Authenticity final
GC>          );
GC>  
GC>    private:
GC> -    Authenticity();
GC> -    ~Authenticity();
GC> +    Authenticity() = default;
GC> +    ~Authenticity() = default;
GC>      Authenticity(Authenticity const&) = delete;
GC>      Authenticity& operator=(Authenticity const&) = delete;
GC>  
GC>      static void ResetCache();
GC>  
GC> -    mutable calendar_date CachedDate_;
GC> +    mutable calendar_date CachedDate_ = calendar_date(jdn_t(0));
GC>  };

 Looking at this, I can't help returning to our discussion about using "="
vs "()" vs "{}" because this type name repetition seems unfortunate and I'd
clearly prefer to apply the following change here:
---------------------------------- >8 --------------------------------------
diff --git a/authenticity.hpp b/authenticity.hpp
index 0ae15ce..2d199a5 100644
--- a/authenticity.hpp
+++ b/authenticity.hpp
@@ -65,7 +65,7 @@ class Authenticity final

     static void ResetCache();

-    mutable calendar_date CachedDate_ = calendar_date(jdn_t(0));
+    mutable calendar_date CachedDate_{jdn_t(0)};
 };

 /// Authenticate production system and its crucial data files.
---------------------------------- >8 --------------------------------------

Wouldn't you?

GC> --- a/fund_data.hpp
GC> +++ b/fund_data.hpp
GC> @@ -38,14 +38,14 @@ class LMI_SO FundInfo final
GC>      friend class FundData;
GC>  
GC>    public:
GC> -    FundInfo();
GC> +    FundInfo() = default;
GC>      FundInfo
GC>          (double             ScalarIMF
GC>          ,std::string const& ShortName
GC>          ,std::string const& LongName
GC>          ,std::string const& gloss = std::string()
GC>          );
GC> -    ~FundInfo();
GC> +    ~FundInfo() = default;
GC>  
GC>      double ScalarIMF() const;
GC>      std::string const& ShortName() const;
GC> @@ -53,17 +53,17 @@ class LMI_SO FundInfo final
GC>      std::string const& gloss() const;
GC>  
GC>    private:
GC> -    double ScalarIMF_;
GC> -    std::string ShortName_;
GC> -    std::string LongName_;
GC> -    std::string gloss_;
GC> +    double      ScalarIMF_ = 0.0;
GC> +    std::string ShortName_ = std::string();
GC> +    std::string LongName_  = std::string();
GC> +    std::string gloss_     = std::string();
GC>  };

 The last 3 new lines above also seem unnecessarily verbose to me, IMHO
this is what default ctors are for and we could just omit the
initialization entirely, but if we wanted to keep it for explicitness sake,
I'd use "std::string ShortName_{};" etc here.

GC> index 3582177..40e1bac 100644
GC> --- a/gpt_view.hpp
GC> +++ b/gpt_view.hpp
GC> @@ -48,8 +48,8 @@ class gpt_mvc_view
GC>      :public MvcView
GC>  {
GC>    public:
GC> -    gpt_mvc_view();
GC> -    ~gpt_mvc_view() override;
GC> +    gpt_mvc_view() = default;
GC> +    ~gpt_mvc_view() override = default;
GC>  
GC>    private:
GC>      // MvcView required implementation.
GC> @@ -64,8 +64,8 @@ class gpt_view final
GC>      friend class gpt_document;
GC>  
GC>    public:
GC> -    gpt_view();
GC> -    ~gpt_view() override;
GC> +    gpt_view() = default;
GC> +    ~gpt_view() override = default;
GC>  
GC>    private:
GC>      gpt_view(gpt_view const&) = delete;
GC> @@ -95,8 +95,8 @@ class gpt_view final
GC>  
GC>      gpt_input& input_data();
GC>  
GC> -    std::string html_content_;
GC> -    wxHtmlWindow* html_window_;
GC> +    std::string html_content_ = std::string("Unable to display results.");

 This is another instance where I'd prefer the use of "{}" (I'm pointing
them all out just in case you decide to change this).

GC> index d301f45..b077c0a 100644
GC> --- a/mec_view.hpp
GC> +++ b/mec_view.hpp
GC> @@ -48,8 +48,8 @@ class mec_mvc_view
GC>      :public MvcView
GC>  {
GC>    public:
GC> -    mec_mvc_view();
GC> -    ~mec_mvc_view() override;
GC> +    mec_mvc_view() = default;
GC> +    ~mec_mvc_view() override = default;
GC>  
GC>    private:
GC>      // MvcView required implementation.
GC> @@ -64,8 +64,8 @@ class mec_view final
GC>      friend class mec_document;
GC>  
GC>    public:
GC> -    mec_view();
GC> -    ~mec_view() override;
GC> +    mec_view() = default;
GC> +    ~mec_view() override = default;
GC>  
GC>    private:
GC>      mec_view(mec_view const&) = delete;
GC> @@ -95,8 +95,8 @@ class mec_view final
GC>  
GC>      mec_input& input_data();
GC>  
GC> -    std::string html_content_;
GC> -    wxHtmlWindow* html_window_;
GC> +    std::string html_content_  = std::string("Unable to display results.");

 And so here is another one.

GC> index a1e74ea..cd1e418 100644
GC> --- a/rounding_rules.hpp
GC> +++ b/rounding_rules.hpp
GC> @@ -46,7 +46,7 @@ class LMI_SO rounding_parameters final
GC>          ,rounding_style     style
GC>          ,std::string const& gloss = std::string()
GC>          );
GC> -    ~rounding_parameters();
GC> +    ~rounding_parameters() = default;
GC>  
GC>      bool operator==(rounding_parameters const&) const;
GC>  
GC> @@ -57,11 +57,12 @@ class LMI_SO rounding_parameters final
GC>      rounding_style            raw_style() const;
GC>  
GC>    private:
GC> -    rounding_parameters();
GC> +    /// Private default ctor, for friends only.
GC> +    rounding_parameters() = default;
GC>  
GC> -    int                decimals_;
GC> -    mce_rounding_style style_   ;
GC> -    std::string        gloss_   ;
GC> +    int                decimals_ = 0;
GC> +    mce_rounding_style style_    = mce_rounding_style(r_indeterminate);
GC> +    std::string        gloss_    = std::string();
GC>  };

 And two last ones.

 Regards,
VZ


reply via email to

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