[lmi-commits] (no subject)

[Greg Chicares]

branch: master
commit 5ba88269d84a3899a7884a951a6a6703fb78b6b5
Author: Gregory W. Chicares <address@hidden>
Date:   Fri Jun 3 00:48:39 2016 +0000

    Rewrite PreferencesModel::IsModified() [447]
    
    It is unnecessary to compare 'UseBuiltinCalculationSummary' explicitly
    because all members including that one are compared in a loop.
    
    Remove some irrelevant comments and a misbegotten defect marker.
---
 preferences_model.cpp |   37 ++++++++++++++++++++-----------------
 preferences_model.hpp |    2 --
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/preferences_model.cpp b/preferences_model.cpp
index f14ce6e..034b545 100644
--- a/preferences_model.cpp
+++ b/preferences_model.cpp
@@ -177,27 +177,24 @@ void PreferencesModel::DoTransmogrify()
 {
 }
 
+/// Determine whether any member has been changed.
+///
+/// Any parse_calculation_summary_columns() diagnostics are repeated
+/// when 'unchanged' is constructed, because the ctor calls Load().
+/// But Load() must be called in that case, because a copy of *(this)
+/// would be identical to itself, frustrating this function's purpose.
+///
+/// The test that compares column selections as a single string is not
+/// superfluous: it serves to detect removal of invalid substrings by
+/// parse_calculation_summary_columns().
+///
+/// This might be renamed operator==(configurable_settings const&),
+/// but that doesn't seem clearer.
+
 bool PreferencesModel::IsModified() const
 {
     PreferencesModel unchanged;
-// CALCULATION_SUMMARY Apparently unneeded: ctor calls Load().
-//    unchanged.Load();
-// Unfortunately, construction of 'unchanged' therefore causes any
-// parse_calculation_summary_columns() diagnostics to be repeated.
-// It would seem better to permit copying, and then use a copy.
-    if(unchanged.UseBuiltinCalculationSummary != UseBuiltinCalculationSummary)
-        {
-        return true;
-        }
     configurable_settings const& z = configurable_settings::instance();
-    if(string_of_column_names() != z.calculation_summary_columns())
-        {
-        return true;
-        }
-    // This test duplicates the preceding one. This one may be what
-    // is ultimately wanted, but for now at least it doesn't detect
-    // parse_calculation_summary_columns()'s removal of invalid
-    // substrings.
     std::vector<std::string>::const_iterator i;
     for(i = member_names().begin(); i != member_names().end(); ++i)
         {
@@ -206,6 +203,12 @@ bool PreferencesModel::IsModified() const
             return true;
             }
         }
+
+    if(string_of_column_names() != z.calculation_summary_columns())
+        {
+        return true;
+        }
+
     return false;
 }
 
diff --git a/preferences_model.hpp b/preferences_model.hpp
index a6685f9..de3a26e 100644
--- a/preferences_model.hpp
+++ b/preferences_model.hpp
@@ -47,8 +47,6 @@ class LMI_SO PreferencesModel
     PreferencesModel();
     virtual ~PreferencesModel();
 
-    // TODO ?? CALCULATION_SUMMARY Use operator==() instead of
-    // IsModified(), once column selections are stored individually.
     bool IsModified() const;
     void Load();
     void Save() const;