[lmi-commits] [lmi] master 10119ee 1/2: Move forward-declared nested class into implementation file

[Greg Chicares]

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

    Move forward-declared nested class into implementation file
---
 wx_table_generator.cpp | 204 +++++++++++++++++++++++++++++++++++++++++++++++++
 wx_table_generator.hpp | 198 ++---------------------------------------------
 2 files changed, 210 insertions(+), 192 deletions(-)

diff --git a/wx_table_generator.cpp b/wx_table_generator.cpp
index 03c4ddd..96a8e50 100644
--- a/wx_table_generator.cpp
+++ b/wx_table_generator.cpp
@@ -27,6 +27,196 @@
 #include "assert_lmi.hpp"
 #include "miscellany.hpp"               // count_newlines(), split_into_lines()
 
+//   is_centered_ is a member variable, initialized in the ctor
+//   is_hidden() is a member function, whose return value is dynamic
+// Should these really be implemented in those two different ways?
+// Wouldn't it be better to treat is_hidden() the same as is_centered_?
+//
+// Is this a struct only because we want its members to be publicly
+// accessible? But their values can also be changed by clients, and
+// isn't that undesirable?
+//
+// In wx_table_generator::do_output_values():
+//   if(align_right_)
+//   if(ci.is_centered_)
+// it seems that right-alignment is a quasi-global, while
+// center-alignment is a column_info data member. Historically, this
+// evolved because right-alignment was recently added to support
+// illustrations, while center-alignment was already used for group
+// quotes. But when code is complex for "historical reasons", it's
+// natural to ask whether it ought to be refactored for uniformity.
+
+// Under what circumstances might columns be hidden, centered, or clipped?
+//
+// General answer:
+//  In principle, all of those are independent. In practice, fixed width
+// columns are centered and variable width columns are clipped and only the
+// former can be hidden. But I don't think this low level class should impose
+// such high level constraints on its use, which is why it doesn't do it.
+//
+// - is_hidden()
+//
+// Apparently used only for group premium quotes, e.g.:
+//
+//             case e_col_total_face_amount:
+//                 if(!has_suppl_amount)
+//                     // Leave the header empty to hide this column.
+//                     break;
+//                 // Fall through
+//             ...
+//                 header = cd.header_;
+//
+// Some columns are conditionally hidden by should_show_column():
+//
+//     // May be overridden to return false if the given column shouldn't be 
shown
+//     // for the specific ledger values (currently used to exclude individual
+//     // columns from composite illustrations).
+//     virtual bool should_show_column(Ledger const& ledger, int column) const
+//
+// but that technique seems to be orthogonal to is_hidden() and used
+// only for illustration PDFs.
+// --No, it's not orthogonal, should_show_column() is used to decide whether
+// the column label should be left empty, making the column hidden.
+//
+//  - is_centered()
+//
+// This seems to be used only in one place:
+//
+//     if(ci.is_centered())
+//         {
+//         // Centre the text for the columns configured to do it.
+//         x_text += (width - dc_.GetTextExtent(s).x) / 2;
+//         }
+//
+// What exactly does it mean for a column to be "centered"? I think this
+// is a different concept than using "center" alignment for cells in a
+// spreadsheet column, which would give, e.g.:
+//     1
+//   11111
+// --No, it's exactly the same concept.
+// In spreadsheet terminology, almost all our columns are numeric, and our
+// numeric columns are right-aligned. But the function is documented thus:
+//
+//     // Return true if this column should be centered, rather than
+//     // left-aligned. Notice that this is ignored for globally right-aligned
+//     // tables.
+//
+// Is it then the case that:
+//  - for illustration PDFs, all columns are right-aligned, and
+//  - is_centered is used only for group quotes, where it really does
+//    mean the same thing as "center" alignment in a spreadsheet
+// ?
+// Answer: yes.
+//  This is indeed not as lucid as I'd like, but the alternative would to
+// modify the PDF quotes code to align all the columns explicitly, which I
+// preferred not to do as part of illustrations work. Maybe now, that this is
+// merged, it's indeed worth changing this.
+//  OTOH, unlike a spreadsheet, this class doesn't have any notion of numeric
+// or text values, so its align_right() method is still useful to globally
+// configure all columns to be right-aligned. Perhaps we could just add a
+// similar align_centre() method and call it from the group PDF quotes code
+// and continue to handle the variable width columns specially by
+// left-aligning them in any case?
+//
+// Apparently is_centered() always returns true (but is ignored)
+// for illustrations, and this comment inside the function body
+// applies to group quotes only:
+//
+//     // Fixed width columns are centered by default, variable width ones
+//     // are not as long strings look better with the default left
+//     // alignment.
+//
+// What sort of columns are not centered?
+// Answer: Variable width ones (only used by PDF quotes).
+//
+// - needs_clipping()
+//
+// And what sort of columns need to be clipped? As currently implemented,
+// this function is the logical negation of is_centered(), so only columns
+// that are not centered need clipping--but what columns are those? Only
+// the "Participant" column on group quotes?
+// Answer: Currently, yes, as it's the only variable width column.
+//
+// Does this all boil down to...
+//  - left-align and clip the group-quote "Participant" column
+//  - center all other group-quote columns
+//  - ignore all these accessors for illustration PDFs
+// ?
+// Answer: yes.
+//
+// And what does 'is_variable_width_' mean? As implemented, it means
+// that the constructor's 'width' argument was zero at the time of
+// construction. Is that in effect just another way of identifying
+// the "Participant" column?
+// Answer:
+//  No, as with "centered" above, it really means what it says: a variable
+// width column is one whose width is not fixed, i.e. not defined by the
+// widest string that can appear in it, but takes all the available space
+// remaining from the other, fixed width columns (in principle, there can be
+// more than one variable width column, even if currently this is not the
+// case).
+// The fundamental distinction is really
+// between fixed and variable width columns: the latter ones are always
+// left-aligned and need to be clipped, while the former ones are either
+// centered or right-aligned (if align_right() was called) and not clipped.
+// And I think things are reasonably simple seen from this point of view and
+// this is how you're supposed to see them, because it's how this class is
+// used, while the various accessors discussed above are just its
+// implementation details.
+
+class wx_table_generator::column_info
+{
+  public:
+    column_info(std::string const& header, int width)
+        :header_(header)
+        ,width_(width)
+        ,is_variable_width_(width == 0)
+        {
+        }
+
+    // A column with empty header is considered to be suppressed and
+    // doesn't appear in the output at all.
+    bool is_hidden() const { return header_.empty(); }
+
+    // Return true if this column should be centered, rather than
+    // left-aligned. Notice that this is ignored for globally right-aligned
+    // tables.
+    bool is_centered() const
+    {
+        // Fixed width columns are centered by default, variable width ones
+        // are not as long strings look better with the default left
+        // alignment.
+        return !is_variable_width_;
+    }
+
+    bool is_variable_width() const
+    {
+        return is_variable_width_;
+    }
+
+    // Return true if the contents of this column needs to be clipped when
+    // outputting it.
+    bool needs_clipping() const
+    {
+        // Variable width columns can have practically unlimited length and
+        // hence overflow into the next column or even beyond and must be
+        // clipped to prevent this from happening. Fixed width columns are
+        // not supposed to overflow anyhow, so clipping them is unnecessary.
+        return is_variable_width_;
+    }
+
+    std::string const header_;
+
+    // Width in pixels. Because the wxPdfDC uses wxMM_POINTS, each
+    // pixel is one point = 1/72 inch.
+    //
+    // Modified directly by wx_table_generator code, hence not const.
+    int width_;
+
+  private:
+    bool const is_variable_width_;
+};
+
 namespace
 {
 
@@ -65,6 +255,10 @@ wx_table_generator::wx_table_generator
     dc_.SetPen(pen);
 }
 
+wx_table_generator::wx_table_generator(wx_table_generator const&) = default;
+
+wx_table_generator::~wx_table_generator() = default;
+
 void wx_table_generator::use_condensed_style()
 {
     row_height_ = char_height_;
@@ -149,6 +343,16 @@ int wx_table_generator::do_get_cell_x(std::size_t column)
     return x;
 }
 
+std::size_t wx_table_generator::columns_count() const
+{
+    return columns_.size();
+}
+
+int wx_table_generator::row_height() const
+{
+    return row_height_;
+}
+
 wxRect wx_table_generator::cell_rect(std::size_t column, int y)
 {
     LMI_ASSERT(column < columns_.size());
diff --git a/wx_table_generator.hpp b/wx_table_generator.hpp
index 4f455e3..2ffaec9 100644
--- a/wx_table_generator.hpp
+++ b/wx_table_generator.hpp
@@ -51,6 +51,10 @@ class wx_table_generator
     // The table has the given total width and starts at the left margin.
     wx_table_generator(wxDC& dc, int left_margin, int total_width);
 
+    wx_table_generator(wx_table_generator const&);
+
+    ~wx_table_generator();
+
     // Adds a column to the table. The total number of added columns determines
     // the cardinality of the 'values' argument in output_row() calls.
     //
@@ -97,10 +101,10 @@ class wx_table_generator
         );
 
     // Return the number of columns.
-    std::size_t columns_count() const {return columns_.size();}
+    std::size_t columns_count() const;
 
     // Return the height of a single table row.
-    int row_height() const {return row_height_;}
+    int row_height() const;
 
     // Return the rectangle containing the cell area.
     wxRect cell_rect(std::size_t column, int y);
@@ -185,194 +189,4 @@ class wx_table_generator
     bool align_right_ = false;
 };
 
-//   is_centered_ is a member variable, initialized in the ctor
-//   is_hidden() is a member function, whose return value is dynamic
-// Should these really be implemented in those two different ways?
-// Wouldn't it be better to treat is_hidden() the same as is_centered_?
-//
-// Is this a struct only because we want its members to be publicly
-// accessible? But their values can also be changed by clients, and
-// isn't that undesirable?
-//
-// In wx_table_generator::do_output_values():
-//   if(align_right_)
-//   if(ci.is_centered_)
-// it seems that right-alignment is a quasi-global, while
-// center-alignment is a column_info data member. Historically, this
-// evolved because right-alignment was recently added to support
-// illustrations, while center-alignment was already used for group
-// quotes. But when code is complex for "historical reasons", it's
-// natural to ask whether it ought to be refactored for uniformity.
-
-// Under what circumstances might columns be hidden, centered, or clipped?
-//
-// General answer:
-//  In principle, all of those are independent. In practice, fixed width
-// columns are centered and variable width columns are clipped and only the
-// former can be hidden. But I don't think this low level class should impose
-// such high level constraints on its use, which is why it doesn't do it.
-//
-// - is_hidden()
-//
-// Apparently used only for group premium quotes, e.g.:
-//
-//             case e_col_total_face_amount:
-//                 if(!has_suppl_amount)
-//                     // Leave the header empty to hide this column.
-//                     break;
-//                 // Fall through
-//             ...
-//                 header = cd.header_;
-//
-// Some columns are conditionally hidden by should_show_column():
-//
-//     // May be overridden to return false if the given column shouldn't be 
shown
-//     // for the specific ledger values (currently used to exclude individual
-//     // columns from composite illustrations).
-//     virtual bool should_show_column(Ledger const& ledger, int column) const
-//
-// but that technique seems to be orthogonal to is_hidden() and used
-// only for illustration PDFs.
-// --No, it's not orthogonal, should_show_column() is used to decide whether
-// the column label should be left empty, making the column hidden.
-//
-//  - is_centered()
-//
-// This seems to be used only in one place:
-//
-//     if(ci.is_centered())
-//         {
-//         // Centre the text for the columns configured to do it.
-//         x_text += (width - dc_.GetTextExtent(s).x) / 2;
-//         }
-//
-// What exactly does it mean for a column to be "centered"? I think this
-// is a different concept than using "center" alignment for cells in a
-// spreadsheet column, which would give, e.g.:
-//     1
-//   11111
-// --No, it's exactly the same concept.
-// In spreadsheet terminology, almost all our columns are numeric, and our
-// numeric columns are right-aligned. But the function is documented thus:
-//
-//     // Return true if this column should be centered, rather than
-//     // left-aligned. Notice that this is ignored for globally right-aligned
-//     // tables.
-//
-// Is it then the case that:
-//  - for illustration PDFs, all columns are right-aligned, and
-//  - is_centered is used only for group quotes, where it really does
-//    mean the same thing as "center" alignment in a spreadsheet
-// ?
-// Answer: yes.
-//  This is indeed not as lucid as I'd like, but the alternative would to
-// modify the PDF quotes code to align all the columns explicitly, which I
-// preferred not to do as part of illustrations work. Maybe now, that this is
-// merged, it's indeed worth changing this.
-//  OTOH, unlike a spreadsheet, this class doesn't have any notion of numeric
-// or text values, so its align_right() method is still useful to globally
-// configure all columns to be right-aligned. Perhaps we could just add a
-// similar align_centre() method and call it from the group PDF quotes code
-// and continue to handle the variable width columns specially by
-// left-aligning them in any case?
-//
-// Apparently is_centered() always returns true (but is ignored)
-// for illustrations, and this comment inside the function body
-// applies to group quotes only:
-//
-//     // Fixed width columns are centered by default, variable width ones
-//     // are not as long strings look better with the default left
-//     // alignment.
-//
-// What sort of columns are not centered?
-// Answer: Variable width ones (only used by PDF quotes).
-//
-// - needs_clipping()
-//
-// And what sort of columns need to be clipped? As currently implemented,
-// this function is the logical negation of is_centered(), so only columns
-// that are not centered need clipping--but what columns are those? Only
-// the "Participant" column on group quotes?
-// Answer: Currently, yes, as it's the only variable width column.
-//
-// Does this all boil down to...
-//  - left-align and clip the group-quote "Participant" column
-//  - center all other group-quote columns
-//  - ignore all these accessors for illustration PDFs
-// ?
-// Answer: yes.
-//
-// And what does 'is_variable_width_' mean? As implemented, it means
-// that the constructor's 'width' argument was zero at the time of
-// construction. Is that in effect just another way of identifying
-// the "Participant" column?
-// Answer:
-//  No, as with "centered" above, it really means what it says: a variable
-// width column is one whose width is not fixed, i.e. not defined by the
-// widest string that can appear in it, but takes all the available space
-// remaining from the other, fixed width columns (in principle, there can be
-// more than one variable width column, even if currently this is not the
-// case).
-// The fundamental distinction is really
-// between fixed and variable width columns: the latter ones are always
-// left-aligned and need to be clipped, while the former ones are either
-// centered or right-aligned (if align_right() was called) and not clipped.
-// And I think things are reasonably simple seen from this point of view and
-// this is how you're supposed to see them, because it's how this class is
-// used, while the various accessors discussed above are just its
-// implementation details.
-
-class wx_table_generator::column_info
-{
-  public:
-    column_info(std::string const& header, int width)
-        :header_(header)
-        ,width_(width)
-        ,is_variable_width_(width == 0)
-        {
-        }
-
-    // A column with empty header is considered to be suppressed and
-    // doesn't appear in the output at all.
-    bool is_hidden() const { return header_.empty(); }
-
-    // Return true if this column should be centered, rather than
-    // left-aligned. Notice that this is ignored for globally right-aligned
-    // tables.
-    bool is_centered() const
-    {
-        // Fixed width columns are centered by default, variable width ones
-        // are not as long strings look better with the default left
-        // alignment.
-        return !is_variable_width_;
-    }
-
-    bool is_variable_width() const
-    {
-        return is_variable_width_;
-    }
-
-    // Return true if the contents of this column needs to be clipped when
-    // outputting it.
-    bool needs_clipping() const
-    {
-        // Variable width columns can have practically unlimited length and
-        // hence overflow into the next column or even beyond and must be
-        // clipped to prevent this from happening. Fixed width columns are
-        // not supposed to overflow anyhow, so clipping them is unnecessary.
-        return is_variable_width_;
-    }
-
-    std::string const header_;
-
-    // Width in pixels. Because the wxPdfDC uses wxMM_POINTS, each
-    // pixel is one point = 1/72 inch.
-    //
-    // Modified directly by wx_table_generator code, hence not const.
-    int width_;
-
-  private:
-    bool const is_variable_width_;
-};
-
 #endif // wx_table_generator_hpp