[lmi-commits] [lmi] master b518132 4/4: Remove the latent defect just added

[Greg Chicares]

branch: master
commit b518132022d0dab59226f584778973265b71e532
Author: Gregory W. Chicares <address@hidden>
Commit: Gregory W. Chicares <address@hidden>

    Remove the latent defect just added
    
    See the two immediately preceding commits.
    
    Abnormal termination is a grave defect, which this commit removes by
    asserting the validity of ctor arguments at the proper moment. To see
    why this option seems better than any other, consider the tradeoffs:
    
     - Constness could be forsaken.
    
     - Constness could be subverted with const_cast.
    
    Or assertions could be made at the proper time by alternative means:
    
     - Push them into a base class, complexifying the design.
    
     - The comma operator won't help, but the conditional operator could
       be used in the ctor-initializer, e.g.:
         ,rows_per_group_ {0 < rows_per_group ? rows_per_group : throw 0;}
       which is less readable than this commit.
    
     - Templates. SFINAE. A barrier to comprehension: overkill here.
    
     - The assertions could be expressed in an macro. Not.
    
     - The possibility of abnormal termination could be disregarded,
       because how could that go wrong?
    
    Join these two lines to produce one URL:
      https://isocpp.org/wiki/faq/ctors
      #initializing-members-from-other-members
    for a discussion of the morality of this commit, which warns of "subtle
    order-dependency errors if someone reorganizes the layout of member
    objects". Yet these calculations are manifestly order-dependent, and
    rearranging the lines of code in the antepenultimate commit would have
    the same effect as rearranging the mem-initializers here. The
    precondition-assertion function could be rewritten to reference the ctor
    arguments directly, e.g.:
       foo::foo(int a, int b)
       :ctor_args_are_sane_ {assert_preconditions(a, b)}
       ,[all other mem-initializers]
    which would arguably alleviate the member-object order dependency, but
    at the cost of creating a dependency on the order of those arguments:
       :ctor_args_are_sane_ {assert_preconditions(b, a)} // Oops!
    while also introducing obscurity and maintenance overhead.
---
 report_table.cpp      | 13 +++++++++----
 report_table.hpp      |  4 ++++
 report_table_test.cpp |  2 --
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/report_table.cpp b/report_table.cpp
index 5a55979..a1864a9 100644
--- a/report_table.cpp
+++ b/report_table.cpp
@@ -175,12 +175,19 @@ std::vector<int> set_column_widths
     return w;
 }
 
-/// Preconditions: 0 <= total_rows && 0 < rows_per_group <= max_lines_per_page
+bool paginator::assert_preconditions() const
+{
+    LMI_ASSERT(0 <= total_rows_);
+    LMI_ASSERT(0 <  rows_per_group_                       );
+    LMI_ASSERT(     rows_per_group_ <= max_lines_per_page_);
+    return true;
+}
 
 paginator::paginator(int total_rows, int rows_per_group, int 
max_lines_per_page)
     :total_rows_         {total_rows}
     ,rows_per_group_     {rows_per_group}
     ,max_lines_per_page_ {max_lines_per_page}
+    ,ctor_args_are_sane_ {assert_preconditions()}
     // "+ 1": blank-line separator after each group.
     ,lines_per_group_    {rows_per_group_ + 1}
     // "+ 1": no blank-line separator after the last group.
@@ -188,9 +195,7 @@ paginator::paginator(int total_rows, int rows_per_group, 
int max_lines_per_page)
     ,rows_per_page_      {rows_per_group_ * groups_per_page_}
     ,page_count_         {1}
 {
-    LMI_ASSERT(0 <= total_rows_);
-    LMI_ASSERT(0 <  rows_per_group_                       );
-    LMI_ASSERT(     rows_per_group_ <= max_lines_per_page_);
+    // Preconditions: see assert_preconditions().
 
     // If there are zero rows of data, then one empty page is wanted.
     if(0 == total_rows_)
diff --git a/report_table.hpp b/report_table.hpp
index b548cf2..930f0f4 100644
--- a/report_table.hpp
+++ b/report_table.hpp
@@ -142,11 +142,15 @@ class LMI_SO paginator
     int page_count() const {return page_count_;}
 
   private:
+    bool assert_preconditions() const;
+
     // Ctor arguments.
     int const total_rows_;
     int const rows_per_group_;
     int const max_lines_per_page_;
 
+    bool const ctor_args_are_sane_ {false};
+
     // Internals in dependency order.
     int const lines_per_group_;
     int const groups_per_page_;
diff --git a/report_table_test.cpp b/report_table_test.cpp
index 68f7e7e..c8e3a1d 100644
--- a/report_table_test.cpp
+++ b/report_table_test.cpp
@@ -440,13 +440,11 @@ void report_table_test::test_paginator()
         );
 
     // Negative number of rows per group.
-#if 0
     BOOST_TEST_THROW
         (paginator(1, -1, 1)
         ,std::runtime_error
         ,lmi_test::what_regex("^Assertion.*failed")
         );
-#endif // 0
 
     // Insufficient room to print even one group.
     BOOST_TEST_THROW