Re: [lmi] [lmi-commits] master a889ef0 2/8: Assert some preconditions

[Vadim Zeitlin]

On Mon,  6 Aug 2018 18:36:23 -0400 (EDT) Greg Chicares <address@hidden> wrote:

GC> branch: master
GC> commit a889ef0ed0d0cabf5d745a9ba8a16c60ef6e1332
GC> Author: Gregory W. Chicares <address@hidden>
GC> Commit: Gregory W. Chicares <address@hidden>
GC> 
GC>     Assert some preconditions
GC>     
GC>     Added assertions that were not observed to fail in fairly extensive
GC>     testing, though it's not necessarily obvious whether the precondition
GC>     should be
GC>       index <  container.size()
GC>     or
GC>       index <= container.size()
GC>     in each particular case.
GC> ---
GC>  wx_table_generator.cpp | 4 ++++
GC>  1 file changed, 4 insertions(+)
GC> 
GC> diff --git a/wx_table_generator.cpp b/wx_table_generator.cpp
GC> index 6f7575f..7c6c0f5 100644
GC> --- a/wx_table_generator.cpp
GC> +++ b/wx_table_generator.cpp
GC> @@ -226,6 +226,7 @@ void wx_table_generator::output_highlighted_cell
GC>      ,std::string const& value
GC>      )
GC>  {
GC> +    LMI_ASSERT(column < all_columns().size());
GC>      if(all_columns().at(column).is_hidden())
GC>          {
GC>          return;

 I don't this precondition is really useful as the call to at() with the
"column" argument already asserts the same precondition almost as
explicitly. However if you prefer to write it like this, then, IMO, it
would make sense to replace the call to at() with operator[].

GC> @@ -334,6 +335,7 @@ int wx_table_generator::separator_line_height() const
GC>  
GC>  wxRect wx_table_generator::text_rect(std::size_t column, int y) const
GC>  {
GC> +    LMI_ASSERT(column <= all_columns().size());
GC>      wxRect z = cell_rect(column, y).Deflate(dc().GetCharWidth(), 0);
GC>      z.Offset(0, (row_height_ - char_height_)/2);
GC>      return z;

 This precondition is too lax, "column" must be a valid index here as it's
passed to cell_rect().

GC> @@ -523,6 +526,7 @@ int wx_table_generator::cell_pos_x(std::size_t column) 
const
GC>  
GC>  wxRect wx_table_generator::cell_rect(std::size_t column, int y) const
GC>  {
GC> +    LMI_ASSERT(column < all_columns().size());
GC>      return wxRect
GC>          (cell_pos_x(column)
GC>          ,y

 Here, again, "column" is used as an argument to at() (in the next line,
omitted from the diff context), so the same remark as above applies.

 Regards,
VZ