[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[lmi-commits] [lmi] master 27d79e2 3/3: Reimplement set_column_widths()
|
From: |
Greg Chicares |
|
Subject: |
[lmi-commits] [lmi] master 27d79e2 3/3: Reimplement set_column_widths() |
|
Date: |
Sun, 19 Aug 2018 11:09:23 -0400 (EDT) |
branch: master
commit 27d79e25ee89c173822b812945fc8ffa3105c33e
Author: Gregory W. Chicares <address@hidden>
Commit: Gregory W. Chicares <address@hidden>
Reimplement set_column_widths()
---
report_table.cpp | 230 ++++++++++++++------------------------------------
report_table.hpp | 4 +-
report_table_test.cpp | 97 +++++++++++++--------
3 files changed, 129 insertions(+), 202 deletions(-)
diff --git a/report_table.cpp b/report_table.cpp
index 69657a1..841aa4c 100644
--- a/report_table.cpp
+++ b/report_table.cpp
@@ -25,9 +25,9 @@
#include "alert.hpp"
#include "assert_lmi.hpp"
-#include "math_functions.hpp" // outward_quotient()
#include "ssize_lmi.hpp"
+#include <algorithm> // min()
#include <numeric> // accumulate()
#include <queue>
#include <utility> // pair
@@ -93,189 +93,89 @@ std::vector<int> apportion(std::vector<int> const& votes,
int total_seats)
/// First, allocate adequate width to each inelastic column; then
/// distribute any excess width left over among elastic columns.
///
-/// The width of each inelastic column reflects:
-/// - a mask like "999,999" (ideally, there would instead be a
-/// quasi-global data structure mapping symbolic column names
-/// to their corresponding headers and maximal widths)
-/// - the header width
-/// - PDF !! the bilateral margins added as a first step below
-/// The margins may be slightly reduced by this function to make
-/// everything fit when it otherwise wouldn't.
+/// Notes on arguments:
+/// max_table_width: page width - page margins
+/// desired_margin: maximum margin for each inelastic column
+/// minimum_margin: minimum margin for every column
+/// all_columns: the width of each inelastic column reflects:
+/// - the header width, and
+/// - a mask like "999,999" (ideally, there would instead be a
+/// quasi-global data structure mapping symbolic column names
+/// to their corresponding headers and maximal widths)
void set_column_widths
- (int total_width
- ,int double_margin
+ (int max_table_width
+ ,int desired_margin
,std::vector<table_column_info>& all_columns
)
-//
-// total_width max table width (page width - page margins)
-// double_margin total left + right margin
-// column_margin spacing on both left and right of column
-// all_columns std::vector<table_column_info>
-// table_column_info::col_width_ is the only member changed
{
- // Use this internally for historical reasons. Formerly, M
- // was passed, and 2M was used; now, 2M is passed, and what's
- // used here is 2M/2. This will soon be reimplemented.
- int const column_margin = double_margin / 2;
- // PDF !! Unconditionally add bilateral margins even though they
- // may conditionally be removed below. This is a questionable
- // design decision; if it is later reversed, then remove the
- // comment about it above the implementation.
- for(auto& i : all_columns)
+ // Soon this will become an argument.
+ int minimum_margin = 1;
+
+ int const cardinality = lmi::ssize(all_columns);
+ int data_width = 0;
+ int n_columns_to_show = 0;
+ for(int j = 0, cum_min_width = 0; j < cardinality; ++j)
{
- if(!i.is_elastic())
+ cum_min_width += all_columns[j].col_width() + minimum_margin;
+ if(cum_min_width <= max_table_width)
{
- i.col_width_ += 2 * column_margin;
+ data_width += all_columns[j].col_width();
+ ++n_columns_to_show;
}
+ else break;
}
- // Number of columns.
- int number_of_columns = 0;
-
- // Number of elastic columns.
- int number_of_elastic_columns = 0;
-
- // Total width of all inelastic columns.
- int total_inelastic_width = 0;
-
- for(auto const& i : all_columns)
+ if(0 == n_columns_to_show)
{
- ++number_of_columns;
-
- if(i.is_elastic())
- {
- ++number_of_elastic_columns;
- }
- else
- {
- total_inelastic_width += i.col_width();
- }
+ alarum() << "Not enough room for even the first column." << LMI_FLUSH;
}
- if(total_width < total_inelastic_width)
+ // These two are boolean, but vector<bool> isn't a Container.
+ std::vector<int> bool_inelastic(n_columns_to_show, false);
+ std::vector<int> bool_elastic (n_columns_to_show, false);
+ for(int j = 0; j < n_columns_to_show; ++j)
{
- // The inelastic columns don't all fit with their original
- // one-em presumptive bilateral margins. Try to make them fit
- // by reducing the margins slightly.
- //
- // The number of pixels that would need to be removed is:
- auto const overflow = total_inelastic_width - total_width;
-
- // Because inelastic columns take more than the available
- // horizontal space, there's no room to fit any elastic
- // columns, so the column-fitting problem is overconstrained.
- // Therefore, don't even try reducing margins if there are any
- // elastic columns.
- if(!number_of_elastic_columns)
- {
-// Also calculate the number of pixels by which it overflows for each column
- // We need to round up in division here to be sure that all columns
- // fit into the available width.
- auto const overflow_per_column = outward_quotient
- (overflow
- ,number_of_columns
- );
-// Now determine whether reducing the margins will make the table fit.
-// If that works, then do it; else don't do it, and print a warning.
-//
-// column_margin is the padding on each side of every column, so
-// the number of pixels between columns, as the table was originally
-// laid out, is two times column_margin--which, as we just determined,
-// was too generous, so we're going to try reducing it.
-// Then this conditional compares
-// the number of pixels by which we must shrink each column, to
-// the number of pixels of padding between columns
-// Reducing the padding is a workable strategy if the desired reduction
-// is less than the padding.
-//
-// Is this as good as it can be, given that coordinates are integers?
-// Answer: Yes--the integers count points, not ems or characters, and
-// typographers wouldn't use any finer unit for this task.
- if(overflow_per_column <= 2 * column_margin)
- {
- // We are going to reduce the total width by more than
- // necessary, in general, because of rounding up above, so
- // compensate for it by giving 1 extra pixel until we run out
- // of these "underflow" pixels.
-// Defect: the number of pixels separating columns might now be zero.
-// '9' is five PDF pixels wide; do we need, say, two pixels between columns?
-//
-// Suggestion: change the
-// overflow_per_column <= column_margin
-// condition to something like:
-// overflow_per_column <= column_margin - 4 // two pixels on each side
-// overflow_per_column <= column_margin - 2 // one pixel on each side
- auto underflow = overflow_per_column * number_of_columns -
overflow;
-
- for(auto& i : all_columns)
- {
- i.col_width_ -= overflow_per_column;
- if(0 < underflow)
- {
- ++i.col_width_;
- --underflow;
- }
- }
+ if(all_columns[j].is_elastic()) {bool_elastic [j] = true;}
+ else {bool_inelastic[j] = true;}
+ }
- // We condensed the columns enough to make them fit, so no need
- // for the warning and we don't have any elastic columns, so
- // we're done.
- return;
- }
-// If overflow_per_column is 1, then column_margin -= 1
-// " " " 2, " " 1
-// " " " 3, " " 2
-// " " " 4, " " 2
-// The 'underflow' logic shrinks columns by the exact number of pixels
-// to use up all the available width. But the column_margin reduction
-// isn't exact due to truncation: when the margin is added (on both sides),
-// is the total of all (margin+column+margin) widths lower than the maximum,
-// so that this is just a small aesthetic issue, or is it too wide, so that
-// not everything fits?
-//
-// Answer:
-// This is an issue of aligning the column text, not of fitting, because the
-// margin is used when positioning the text inside the column width. And the
-// width is correct, so the worst that can happen here is that the text is
-// offset by 0.5 pixels -- but, of course, if we rounded it down, it would be
-// offset by 0.5 pixels in the other direction. So maybe we should write
-//
-// column_margin -= overflow_per_column / 2;
-//
-// just because it's shorter and not necessarily worse (nor better).
- }
+ int const residue = max_table_width - data_width;
+ LMI_ASSERT(0 <= residue);
+
+ // Apportion any residue among inelastic columns, up to the number
+ // of such columns times the desired_margin argument.
+ int const n_inelastic = std::accumulate(bool_inelastic.begin(),
bool_inelastic.end(), 0);
+ int const residue_inelastic = std::min(residue, n_inelastic *
desired_margin);
+ LMI_ASSERT(0 <= residue_inelastic);
+ std::vector<int> const delta_inelastic = apportion(bool_inelastic,
residue_inelastic);
+ // That part of the residue should always be fully consumed.
+ LMI_ASSERT(residue_inelastic == std::accumulate(delta_inelastic.begin(),
delta_inelastic.end(), 0));
+
+ // Apportion all remaining residue, if any, among elastic columns.
+ int const residue_elastic = residue - residue_inelastic;
+ LMI_ASSERT(0 <= residue_elastic);
+ std::vector<int> const delta_elastic = apportion(bool_elastic,
residue_elastic);
+
+ std::vector<int> w(cardinality);
+ for(int j = 0; j < n_columns_to_show; ++j)
+ {
+ w[j] = all_columns[j].col_width() + delta_inelastic[j] +
delta_elastic[j];
+ }
+ if(cardinality != n_columns_to_show)
+ {
warning()
- << "Not enough space for all " << number_of_columns << " columns."
- << "\nPrintable width is " << total_width << " points."
- << "\nData alone require " << total_inelastic_width - 2 *
column_margin * number_of_columns
- << " points without any margins for legibility."
- << "\nColumn margins of " << column_margin << " points on both
sides"
- << " would take up " << 2 * column_margin * number_of_columns << "
additional points."
- << LMI_FLUSH
+ << "Printing only the first " << n_columns_to_show
+ << " columns: not enough room for all " << cardinality << "."
+ << std::flush
;
- return;
}
-
- // Lay out elastic columns in whatever space is left over after
- // accounting for all inelastic columns. Clip to make them fit.
- //
- // If there's more than enough space for them, then expand them
- // to consume all available space.
- if(number_of_elastic_columns)
+ // Soon the all_columns argument will be passed by const reference,
+ // and w will be returned by value; until then...
+ for(int j = 0; j < cardinality; ++j)
{
- int const width_of_each_elastic_column = outward_quotient
- (total_width - total_inelastic_width
- ,number_of_elastic_columns
- );
-
- for(auto& i : all_columns)
- {
- if(i.is_elastic())
- {
- i.col_width_ = width_of_each_elastic_column;
- }
- }
+ all_columns[j].col_width_ = w[j];
}
+// return w;
}
diff --git a/report_table.hpp b/report_table.hpp
index e8faa35..fc7803b 100644
--- a/report_table.hpp
+++ b/report_table.hpp
@@ -106,8 +106,8 @@ class LMI_SO table_column_info
std::vector<int> LMI_SO apportion(std::vector<int> const& votes, int seats);
void LMI_SO set_column_widths
- (int total_width
- ,int double_margin
+ (int max_table_width
+ ,int desired_margin
,std::vector<table_column_info>& all_columns
);
diff --git a/report_table_test.cpp b/report_table_test.cpp
index 3ddc211..573a243 100644
--- a/report_table_test.cpp
+++ b/report_table_test.cpp
@@ -141,6 +141,19 @@ void report_table_test::test_apportion()
std::vector<int> const votes6 = {5, 5, 5};
std::vector<int> const seats6 = {3, 2, 2};
BOOST_TEST(seats6 == apportion(votes6, 7));
+
+ // Test with boolean vectors. This special case of the general
+ // algorithm is suitable for apportioning marginal space evenly
+ // among columns in a table.
+
+ // All space apportioned--first column gets more.
+ BOOST_TEST(std::vector<int>({3, 2, 2}) == apportion({1, 1, 1}, 7));
+
+ // Set apportionable space so that all columns get the same.
+ BOOST_TEST(std::vector<int>({2, 2, 2}) == apportion({1, 1, 1}, 6));
+
+ // Set boolean vectors so that some columns get none.
+ BOOST_TEST(std::vector<int>({0, 5, 0}) == apportion({0, 1, 0}, 5));
}
void report_table_test::test_bloat()
@@ -173,57 +186,54 @@ void report_table_test::test_generally()
std::vector<table_column_info> v;
std::vector<int> expected;
- // Width with default margins (12) = maximum available page width.
+ // Just enough room for all data with desired margins.
v = bloat({1, 2, 3}, {0, 0, 0});
set_column_widths(12, 2, v);
expected = {3, 4, 5};
BOOST_TEST(widths(v) == expected);
- // Same columns: same layout, even if page is much wider (99).
+ // Same columns: same layout, even if page is much wider.
v = bloat({1, 2, 3}, {0, 0, 0});
set_column_widths(99, 2, v);
BOOST_TEST(widths(v) == expected);
// Same columns, but inadequate page width.
- // PDF !! Begin section subject to revision.
-
// Tests in this section are overconstrained in that they don't
- // have enough room to print all inelastic columns with bilateral
- // margins of at least one point.
- //
- // For now, what's tested is the actual behavior of current code
- // in the absence of elastic columns, viz.:
- // - for vector input column widths W[i], speculatively define
- // X[i] = W[i] + input margin (a positive scalar)
- // - if sum(X) < available page width
- // then set W=X
- // - if sum(W) < available page width < sum(X)
- // then apportion any available margin among columns in W
- // and issue no diagnostic even if some columns get no margin
- // - else, i.e., if available page width < sum(W) < sum(X)
- // then set W=X
- // and issue a diagnostic
-
- std::vector<int> actual;
+ // have enough room to print all inelastic columns with a margin
+ // of at least one point.
v = bloat({1, 2, 3}, {0, 0, 0});
set_column_widths(11, 2, v);
- actual = {3, 4, 4};
- BOOST_TEST(widths(v) == actual);
+ expected = {3, 4, 4};
+ BOOST_TEST(widths(v) == expected);
+ // Just enough room for all data with minimum margins.
v = bloat({1, 2, 3}, {0, 0, 0});
- set_column_widths( 6, 2, v);
- actual = {1, 2, 3};
- BOOST_TEST(widths(v) == actual);
+ set_column_widths( 9, 2, v);
+ expected = {2, 3, 4};
+ BOOST_TEST(widths(v) == expected);
- // Warning given here:
+ // Not enough room for all data with minimum margins.
v = bloat({1, 2, 3}, {0, 0, 0});
+ std::cout << "Expect a diagnostic (printing 2/3 columns):\n ";
+ set_column_widths( 8, 2, v);
+ expected = {3, 4, 0};
+ BOOST_TEST(widths(v) == expected);
+
+ // Not enough room for all data, even with no margins at all.
+ v = bloat({1, 2, 3}, {0, 0, 0});
+ std::cout << "Expect a diagnostic (printing 2/3 columns):\n ";
set_column_widths( 5, 2, v);
- actual = {3, 4, 5};
- BOOST_TEST(widths(v) == actual);
+ expected = {2, 3, 0};
+ BOOST_TEST(widths(v) == expected);
- // PDF !! End section subject to revision.
+ // Not enough room for even the first column.
+ BOOST_TEST_THROW
+ (set_column_widths(1, 2, v)
+ ,std::runtime_error
+ ,"Not enough room for even the first column."
+ );
// An elastic column occupies all available space not claimed by
// inelastic columns...
@@ -241,10 +251,28 @@ void report_table_test::test_generally()
// Multiple elastic columns apportion all unclaimed space among
// themselves.
- v = bloat({1, 2, 0, 3}, {1, 0, 1, 0});
+ v = bloat({0, 2, 0, 3}, {1, 0, 1, 0});
set_column_widths(99, 2, v);
expected = {45, 4, 45, 5};
BOOST_TEST(widths(v) == expected);
+
+ // Same, but with nonzero width specified for one elastic column.
+ v = bloat({1, 2, 0, 3}, {1, 0, 1, 0});
+ set_column_widths(99, 2, v);
+ expected = {46, 4, 44, 5};
+ BOOST_TEST(widths(v) == expected);
+
+ // Elastic columns only.
+ v = bloat({10, 20, 30}, {1, 1, 1});
+ set_column_widths(99, 2, v);
+ expected = {23, 33, 43};
+ BOOST_TEST(widths(v) == expected);
+
+ // Same columns, but all inelastic.
+ v = bloat({10, 20, 30}, {0, 0, 0});
+ set_column_widths(99, 2, v);
+ expected = {12, 22, 32};
+ BOOST_TEST(widths(v) == expected);
}
/// Test data for an actual group quote.
@@ -356,15 +384,14 @@ void report_table_test::test_illustration()
,{"", 50, oe_right, oe_inelastic}
};
-std::cout << "[Expect a multiline..." << std::endl;
+ std::cout << "Expect a diagnostic (printing 11/12 columns):\n ";
set_column_widths(total_width, default_margin, v);
-std::cout << "...warning message.]" << std::endl;
// Today, two times the default margin is added to each column,
// even though the data cannot fit.
std::vector<int> const observed = widths(v);
- std::vector<int> const expected = {64, 64, 64, 64, 64, 64, 64, 64, 64, 64,
64, 64};
- BOOST_TEST(total_width < sum(expected));
+ std::vector<int> const expected = {53, 53, 53, 53, 52, 52, 52, 52, 52, 52,
52, 0};
+ BOOST_TEST(total_width == sum(expected));
BOOST_TEST(observed == expected);
#if 0 // Doesn't throw today, but might someday.