[lmi-commits] [lmi] master 45b086a 13/13: Simplify logic

[Greg Chicares]

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

    Simplify logic
    
    Expressed the required control flow simply: at time of construction,
    store every column's metadata and calculate its width.
    
    Renamed add_column() to avoid the confusion described here:
      https://lists.nongnu.org/archive/html/lmi/2018-04/msg00111.html
---
 wx_table_generator.cpp | 62 ++++++++++++++------------------------------------
 wx_table_generator.hpp | 16 ++++---------
 2 files changed, 22 insertions(+), 56 deletions(-)

diff --git a/wx_table_generator.cpp b/wx_table_generator.cpp
index d7a95ce..781bef1 100644
--- a/wx_table_generator.cpp
+++ b/wx_table_generator.cpp
@@ -242,13 +242,13 @@ wx_table_generator::wx_table_generator
     ,char_height_(dc_.GetCharHeight())
     ,row_height_((4 * char_height_ + 2) / 3) // Arbitrarily use 1.333 line 
spacing.
     ,column_margin_(dc_.GetTextExtent("M").x)
-    ,column_widths_already_computed_(false)
     ,max_header_lines_(1)
 {
     for(auto const& i : vc)
         {
-        add_column(i.header, i.widest_text);
+        enroll_column(i.header, i.widest_text);
         }
+    compute_column_widths();
 
     // Set a pen with 0 width to get the thin lines, and round cap style for 
the
     // different segments drawn in do_output_values() to seamlessly combine
@@ -291,8 +291,10 @@ void wx_table_generator::align_right()
     align_right_ = true;
 }
 
-/// Adds a column to the table. The total number of added columns determines
-/// the cardinality of the 'values' argument in output_row() calls.
+/// Indicate an intention to include a column by storing its metadata.
+///
+/// The total number of columns thus enrolled determines the cardinality
+/// of the 'values' argument in output_row() calls.
 ///
 /// Providing an empty header suppresses the column display, while still
 /// taking it into account in output_row(), providing a convenient way to
@@ -300,18 +302,16 @@ void wx_table_generator::align_right()
 ///
 /// Each column must either have a fixed width, specified as the width of
 /// the longest text that may appear in this column, or be expandable
-/// meaning that the rest of the page width is allocated to it which will be
-/// the case if widest_text is empty.
+/// meaning that the rest of the page width is allocated to it which will
+/// be the case if widest_text is empty.
 ///
 /// Notice that column headers may be multiline strings.
 
-void wx_table_generator::add_column
+void wx_table_generator::enroll_column
     (std::string const& header
     ,std::string const& widest_text
     )
 {
-    LMI_ASSERT(!column_widths_already_computed_);
-
     // If a column's header is empty, then it is to be hidden--and its
     // width must be initialized to zero, because other member functions
     // calculate total width by accumulating the widths of all columns,
@@ -370,8 +370,6 @@ void wx_table_generator::do_output_vert_separator(int x, 
int y1, int y2)
 
 int wx_table_generator::do_get_cell_x(std::size_t column)
 {
-    do_compute_column_widths();
-
     int x = left_margin_;
     for(std::size_t col = 0; col < column; ++col)
         {
@@ -392,16 +390,12 @@ int wx_table_generator::row_height() const
 
 wxRect wx_table_generator::cell_rect(std::size_t column, int y)
 {
-    LMI_ASSERT(column < all_columns().size());
-
-    // Note: call do_get_cell_x() here and not from the wxRect ctor arguments
-    // list to ensure that the column width is initialized before it is used
-    // below (because calling do_get_cell_x() calculates column widths as a
-    // side effect, but function arguments are evaluated in unspecified
-    // order).
-    int const x = do_get_cell_x(column);
-
-    return wxRect(x, y, all_columns().at(column).col_width(), row_height_);
+    return wxRect
+        (do_get_cell_x(column)
+        ,y
+        ,all_columns().at(column).col_width()
+        ,row_height_
+        );
 }
 
 /// Return the rectangle adjusted for the text contents of the cell: it is
@@ -419,7 +413,6 @@ wxRect wx_table_generator::text_rect(std::size_t column, 
int y)
 // class members used, mutably or immutably:
 //
 // const    total_width_
-// mutable  column_widths_already_computed_
 // mutable  column_margin_
 // mutable  all_columns_
 //   i.e. std::vector<column_info> all_columns_;
@@ -432,8 +425,6 @@ wxRect wx_table_generator::text_rect(std::size_t column, 
int y)
     // ctor parameter:
     // max table width (page width minus horizontal page margins)
 // const    total_width_
-    // Used to prevent this function from being called more than once.
-// mutable  column_widths_already_computed_
     // spacing on both left and right of column
     // initialized in ctor to # pixels in one em: (dc_.GetTextExtent("M").x)
     // changed in this function and nowhere else
@@ -442,24 +433,9 @@ wxRect wx_table_generator::text_rect(std::size_t column, 
int y)
 // mutable  all_columns_
 
 /// Compute column widths.
-///
-/// This function must be called after the last time add_column() is
-/// called, and before the first time that the column widths it sets
-/// are used. It is assumed to be fairly expensive, so that it should
-/// be called only once. There seems to be no simple way to impose
-/// those synchronization requirements upon clients, so
-///  - add_column() asserts that this function hasn't yet been called;
-///  - this function exits early if it has already been called.
-/// Thus, any violation of the first part of this contract is detected
-/// at run time. It is hoped that this function is called in enough
-/// places to fulfill the second part of the contract.
-
-void wx_table_generator::do_compute_column_widths()
-{
-    if(column_widths_already_computed_) return;
-
-    column_widths_already_computed_ = true;
 
+void wx_table_generator::compute_column_widths()
+{
     // Number of non-hidden columns.
     int num_columns = 0;
 
@@ -779,8 +755,6 @@ void wx_table_generator::output_horz_separator
     LMI_ASSERT(begin_column < end_column);
     LMI_ASSERT(end_column <= all_columns().size());
 
-    do_compute_column_widths();
-
     int const x1 = do_get_cell_x(begin_column);
 
     int x2 = x1;
@@ -814,8 +788,6 @@ void wx_table_generator::output_header
             return;
         }
 
-    do_compute_column_widths();
-
     wxDCFontChanger header_font_setter(dc_);
     if(use_bold_headers_)
         {
diff --git a/wx_table_generator.hpp b/wx_table_generator.hpp
index f9f69c4..abf16b9 100644
--- a/wx_table_generator.hpp
+++ b/wx_table_generator.hpp
@@ -43,11 +43,9 @@ struct column_parameters
 
 /// Simplifies outputting tabular data on wxDC.
 ///
-/// To create a table, columns must be initialized first by calling
-/// add_column() for each of them once. After this, output_header() and
-/// output_row() can be called reusing the same pos_y argument which contains
-/// the coordinate of the top of the header or row to output and is updated to
-/// correspond to the value for the next row by these functions.
+/// output_header() and output_row() reuse the same pos_y argument (which
+/// is initially the coordinate of the top of the header or row to output)
+/// and update it to designate the next row to be written.
 ///
 /// The life time of the specified wxDC must be greater than the life time
 /// of this object itself and nothing should be using it while this object
@@ -108,8 +106,8 @@ class wx_table_generator
     void align_right();
 
   private:
-    void add_column(std::string const& header, std::string const& widest_text);
-    void do_compute_column_widths();
+    void enroll_column(std::string const& header, std::string const& 
widest_text);
+    void compute_column_widths();
 
     wxFont get_header_font() const;
 
@@ -141,10 +139,6 @@ class wx_table_generator
 
     std::vector<column_info> all_columns_;
 
-    // Initially false, set to true after do_compute_column_widths()
-    // has been called to make all column_info::col_width_ values valid.
-    bool column_widths_already_computed_;
-
     // Maximal number of lines in any column header, initially 1 but can be
     // higher if multiline headers are used.
     std::size_t max_header_lines_;