[lmi-commits] [6012] Explain why some code and certain defect markers will soon be removed

[Greg Chicares]

Revision: 6012
          http://svn.sv.gnu.org/viewvc/?view=rev&root=lmi&revision=6012
Author:   chicares
Date:     2014-11-01 12:19:17 +0000 (Sat, 01 Nov 2014)
Log Message:
-----------
Explain why some code and certain defect markers will soon be removed

Modified Paths:
--------------
    lmi/trunk/census_view.cpp
    lmi/trunk/census_view.hpp

Modified: lmi/trunk/census_view.cpp
===================================================================
--- lmi/trunk/census_view.cpp   2014-10-31 12:49:41 UTC (rev 6011)
+++ lmi/trunk/census_view.cpp   2014-11-01 12:19:17 UTC (rev 6012)
@@ -69,6 +69,9 @@
 {
 // TODO ?? Add description and unit tests; consider relocating,
 // and include "miscellany.hpp" only in ultimate location.
+// ^ Remove comment. The function name is already descriptive enough,
+// and there's no good reason to move it to a "library" because it's
+// wanted only in this TU.
 std::string insert_spaces_between_words(std::string const& s)
 {
     std::string r;
@@ -921,6 +924,9 @@
     while(i != class_parms().end())
         {
         // TODO ?? Add an any_member operator== instead.
+// ^ Remove comment. At one time, adding operator==(std::string)
+// seemed like a good idea, but that suggestion was discarded here:
+//   
http://svn.savannah.nongnu.org/viewvc/lmi/trunk/any_member.hpp?root=lmi&r1=1593&r2=1758
         if(class_name == (*i)["EmployeeClass"].str())
             {
             return &*i;
@@ -1009,12 +1015,14 @@
         if(wxYES == z)
             {
             // TODO ?? Reserved for grid implementation.
+// ^ Remove comment. See header: this whole function will be removed.
             }
         }
     return false;
 }
 
 // TODO ?? Reserved for a grid implementation.
+// ^ Remove comment. See header.
 int CensusView::selected_column()
 {
     return 0;
@@ -1133,7 +1141,11 @@
     //   if class defaults changed: all cells in the class.
 
     // TODO ?? temp string for new value, eeclass?
+// ^ Remove comment. This is a dubious micro-optimization.
     // TODO ?? combine class and indv vectors for case changes?
+// ^ Remove comment. Merging those two vectors might make a small part
+// of the code more concise, but would cost too much, and there's no
+// convenient way to merge their iterators.
 
     std::vector<std::string> headers_of_changed_parameters;
     std::vector<std::string> const& 
all_headers(case_parms()[0].member_names());
@@ -1496,6 +1508,7 @@
     illustrator z(emission);
     if(!z(base_filename(), cell_parms()))
         {
+        // Cancelled during run_census::operator().
         return false;
         }
 

Modified: lmi/trunk/census_view.hpp
===================================================================
--- lmi/trunk/census_view.hpp   2014-10-31 12:49:41 UTC (rev 6011)
+++ lmi/trunk/census_view.hpp   2014-11-01 12:19:17 UTC (rev 6012)
@@ -124,21 +124,38 @@
         );
 
     bool is_invalid();
+// ^ Remove. Purpose was to prevent operations that shouldn't be
+// allowed when all_changes_have_been_validated_ (q.v.) is false.
 
     int selected_column();
+// ^ Remove. Might have been useful for a grid implementation, but
+// the dataview implementation does not provide column editing.
     int selected_row();
 
     void update_class_names();
 
     bool all_changes_have_been_validated_;
+// ^ Remove. While lmi uses MVC, its predecessor used doc-view, which
+// didn't distinguish View from Controller; worse, it performed
+// validation in the View when it should have been done in the Model.
+// Even worse, validation only occurred in a tabbed dialog akin to
+// lmi's IllustrationView--but direct drill-down editing was provided
+// in an interface similar to lmi's CensusView, where validation was
+// performed only by opening a tabbed dialog for each cell and cycling
+// through its tabs, after which this flag was set.
 
     bool autosize_columns_;
 
     bool composite_is_available_;
+// ^ Remove. Purpose was to cache composite result so that it would
+// be cheap to view. However, it is produced exactly when it is to be
+// viewed; and there's no reason to create an identical second view.
 
     boost::shared_ptr<Ledger const> composite_ledger_;
 
     bool was_cancelled_;
+// ^ Remove. Always false. Purpose was to prevent showing composite if
+// composite run had been cancelled.
 
     wxDataViewCtrl* list_window_;
     wxObjectDataPtr<CensusViewDataViewModel> list_model_;