[lmi-commits] [6386] Rename a function, explaining away what had seemed to be magic

[Greg Chicares]

Revision: 6386
          http://svn.sv.gnu.org/viewvc/?view=rev&root=lmi&revision=6386
Author:   chicares
Date:     2015-11-04 05:10:41 +0000 (Wed, 04 Nov 2015)
Log Message:
-----------
Rename a function, explaining away what had seemed to be magic

Modified Paths:
--------------
    lmi/trunk/accountvalue.cpp
    lmi/trunk/custom_io_1.cpp
    lmi/trunk/ihs_acctval.cpp
    lmi/trunk/input.cpp
    lmi/trunk/input.hpp

Modified: lmi/trunk/accountvalue.cpp
===================================================================
--- lmi/trunk/accountvalue.cpp  2015-11-04 04:16:22 UTC (rev 6385)
+++ lmi/trunk/accountvalue.cpp  2015-11-04 05:10:41 UTC (rev 6386)
@@ -33,7 +33,7 @@
 #include "database.hpp"
 #include "dbnames.hpp"
 #include "death_benefits.hpp"
-#include "input.hpp"                    // magically_rectify()
+#include "input.hpp"                    // consummate()
 #include "interest_rates.hpp"
 #include "ledger.hpp"
 #include "ledger_invariant.hpp"
@@ -116,7 +116,7 @@
 
 //============================================================================
 AccountValue::AccountValue(Input const& input)
-    :BasicValues       (Input::magically_rectify(input))
+    :BasicValues       (Input::consummate(input))
     ,DebugFilename     ("anonymous.monthly_trace")
     ,ledger_(new Ledger(BasicValues::GetLength(), BasicValues::ledger_type(), 
BasicValues::nonillustrated(), BasicValues::no_can_issue(), false))
     ,ledger_invariant_ (new LedgerInvariant(BasicValues::GetLength()))

Modified: lmi/trunk/custom_io_1.cpp
===================================================================
--- lmi/trunk/custom_io_1.cpp   2015-11-04 04:16:22 UTC (rev 6385)
+++ lmi/trunk/custom_io_1.cpp   2015-11-04 05:10:41 UTC (rev 6386)
@@ -286,7 +286,7 @@
     z["AgentPhone"                 ] = AgentPhone;
     z["AgentId"                    ] = AgentLicense;
 
-    z = Input::magically_rectify(z);
+    z = Input::consummate(z);
 
     // For internal testing only, if "AutoClose" has this special
     // value, then write input in lmi's usual format.

Modified: lmi/trunk/ihs_acctval.cpp
===================================================================
--- lmi/trunk/ihs_acctval.cpp   2015-11-04 04:16:22 UTC (rev 6385)
+++ lmi/trunk/ihs_acctval.cpp   2015-11-04 05:10:41 UTC (rev 6386)
@@ -37,7 +37,7 @@
 #include "death_benefits.hpp"
 #include "ihs_irc7702.hpp"
 #include "ihs_irc7702a.hpp"
-#include "input.hpp"                    // magically_rectify()
+#include "input.hpp"                    // consummate()
 #include "interest_rates.hpp"
 #include "ledger.hpp"
 #include "ledger_invariant.hpp"
@@ -92,7 +92,7 @@
 
 //============================================================================
 AccountValue::AccountValue(Input const& input)
-    :BasicValues           (Input::magically_rectify(input))
+    :BasicValues           (Input::consummate(input))
     ,DebugFilename         ("anonymous.monthly_trace")
     ,Debugging             (false)
     ,Solving               (mce_solve_none != 
BasicValues::yare_input_.SolveType)

Modified: lmi/trunk/input.cpp
===================================================================
--- lmi/trunk/input.cpp 2015-11-04 04:16:22 UTC (rev 6385)
+++ lmi/trunk/input.cpp 2015-11-04 05:10:41 UTC (rev 6386)
@@ -29,7 +29,7 @@
 #include "input.hpp"
 
 #include "alert.hpp"
-#include "database.hpp" // Needed only for database_'s dtor.
+#include "database.hpp"                 // 
product_database::~product_database()
 #include "timer.hpp"
 
 /// Values are initialized by UDT defaults where appropriate, and here
@@ -533,7 +533,61 @@
         }
 }
 
-Input Input::magically_rectify(Input const& original)
+/// Make sure input is consistent before using it.
+///
+/// Classes derived from MvcModel permit changing any of their member
+/// data at any time. They provide member functions for maintaining
+/// consistency among such values. When should those member functions
+/// be called, and by whom?
+///
+/// One possible answer is that MvcModel should maintain its own
+/// consistency, by calling those member functions synchronously. But
+/// consider this case:
+///   Input i;
+///   i["ProductName"] = some_product_with_no_term_rider; // initially
+///   i["TermRider"] = "Yes";
+///   i["ProductName"] = some_product_that_offers_a_term_rider;
+/// In the synchronous case, the middle assignment would be rejected;
+/// there would have to be some mechanism for reporting that, because
+/// silent failure is clearly undesirable. It would not be possible to
+/// read values from an input file without sorting them carefully in
+/// advance; but that would be a daunting task.
+///
+/// In the GUI, MvcController calls those member functions frequently
+/// enough to maintain consistency synchronously. That's appropriate
+/// for a GUI: in the example above, an end user would not expect to
+/// elect a term rider without first selecting a product that permits
+/// it. What would be a daunting task for the program is natural and
+/// obvious for an end user familiar with the problem domain who is
+/// guided by active, visual control enablement.
+///
+/// In the asynchronous, non-GUI case, consistency must be ensured by
+/// calling those member functions after the last change to member
+/// data. That has to be the client's responsibility. For example,
+/// AccountValue::AccountValue() calls this function just before it
+/// creates an illustration.
+///
+/// Consider running a "custom_io_1" input file with DOB 1961-12-31,
+/// specifying <AutoClose>X</AutoClose> to generate an equivalent
+/// '.ill' file. For "custom_io_1" input, DOB governs. In this case,
+/// the corresponding issue age has exceeded 45 for as long as lmi
+/// has existed, yet the '.ill' file contains <IssueAge>45</IssueAge>
+/// (as of 20151102T2351Z, revision 6384) because the default age is
+/// inconsistent with the DOB. That's fixed when an illustration is
+/// run: AccountValue::AccountValue() calls this function, which calls
+/// MvcModel::Reconcile(), which calls MvcModel::Transmogrify(), which
+/// calls Input::DoTransmogrify(), which then sets <IssueAge>, so the
+/// illustration is correct--only because this function was called.
+///
+/// That specific issue in "custom_io_1" code was addressed by calling
+/// this function explicitly (20151104T0416Z, revision 6385). However,
+/// similar issues may exist or may arise elsewhere, and are already
+/// present in some old regression-testing files, so the only way to
+/// be sure this function is called before producing an illustration
+/// is to call it at an appropriate chokepoint; that's why it's called
+/// explicitly by AccountValue::AccountValue().
+
+Input Input::consummate(Input const& original)
 {
     Input z(original);
 
@@ -552,7 +606,7 @@
         z["FundChoiceType"] = "Override fund";
         }
 
-    z.Reconcile(); // TODO ?? Necessary only for problematic old cases.
+    z.Reconcile();
     z.RealizeAllSequenceInput();
     z.make_term_rider_consistent();
 

Modified: lmi/trunk/input.hpp
===================================================================
--- lmi/trunk/input.hpp 2015-11-04 04:16:22 UTC (rev 6385)
+++ lmi/trunk/input.hpp 2015-11-04 05:10:41 UTC (rev 6386)
@@ -148,7 +148,7 @@
 
     std::vector<std::string> RealizeAllSequenceInput(bool report_errors = 
true);
 
-    static Input magically_rectify(Input const&);
+    static Input consummate(Input const&);
 
   private:
     void AscribeMembers();
@@ -393,7 +393,7 @@
     numeric_sequence         TaxBracket                      ;
     numeric_sequence         ProjectedSalary                 ;
     specamt_sequence         SpecifiedAmount                 ;
-    // For brevity, "SupplementalAmount" would seem better.
+    // INPUT !! For brevity, "SupplementalAmount" would seem better.
     specamt_sequence         SupplementalSpecifiedAmount     ;
     dbo_sequence             DeathBenefitOption              ;
     payment_sequence         Payment                         ;