[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[lmi-commits]  Explain why some code and certain defect markers wi
[lmi-commits]  Explain why some code and certain defect markers will soon be removed
Sat, 01 Nov 2014 12:19:18 +0000
Date: 2014-11-01 12:19:17 +0000 (Sat, 01 Nov 2014)
Explain why some code and certain defect markers will soon be removed
--- 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)
@@ -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:
if(class_name == (*i)["EmployeeClass"].str())
@@ -1009,12 +1015,14 @@
if(wxYES == z)
// TODO ?? Reserved for grid implementation.
+// ^ Remove comment. See header: this whole function will be removed.
// TODO ?? Reserved for a grid implementation.
+// ^ Remove comment. See header.
@@ -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.
@@ -1496,6 +1508,7 @@
+ // Cancelled during run_census::operator().
--- 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 @@
+// ^ Remove. Purpose was to prevent operations that shouldn't be
+// allowed when all_changes_have_been_validated_ (q.v.) is false.
+// ^ Remove. Might have been useful for a grid implementation, but
+// the dataview implementation does not provide column editing.
+// ^ 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.
+// ^ 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_;
+// ^ Remove. Always false. Purpose was to prevent showing composite if
+// composite run had been cancelled.
|[Prev in Thread]
||[Next in Thread]|
- [lmi-commits]  Explain why some code and certain defect markers will soon be removed,
Greg Chicares <=