Re: [lmi] [lmi-commits] master c59d263 4/5: Replace C-style array with vector

[Vadim Zeitlin]

On Wed, 25 Apr 2018 10:53:48 -0400 (EDT) Greg Chicares <address@hidden> wrote:

GC> branch: master
GC> commit c59d2632f6019ea31607fa2cecba69b707fb8ed9
GC> Author: Gregory W. Chicares <address@hidden>
GC> Commit: Gregory W. Chicares <address@hidden>
GC> 
GC>     Replace C-style array with vector
GC> ---
GC>  group_quote_pdf_gen_wx.cpp | 5 +++--
GC>  1 file changed, 3 insertions(+), 2 deletions(-)
GC> 
GC> diff --git a/group_quote_pdf_gen_wx.cpp b/group_quote_pdf_gen_wx.cpp
GC> index f302e36..1076110 100644
GC> --- a/group_quote_pdf_gen_wx.cpp
GC> +++ b/group_quote_pdf_gen_wx.cpp
GC> @@ -346,7 +346,8 @@ class group_quote_pdf_generator_wx
GC>  
GC>      struct row_data
GC>          {
GC> -        std::string output_values[e_col_max];
GC> +        row_data() {output_values.resize(e_col_max);}
GC> +        std::vector<std::string> output_values;
GC>          };
GC>      std::vector<row_data> rows_;

 Why not just use

        struct row_data
            {
            std::vector<std::string> output_values{e_col_max};
            };

? Although, to be honest, I'd really prefer to used std::array here because
the main point of using an array and not a vector here originally was to
show that this array is _not_ resizable (this code predates the switch to
C++11 in lmi, so std::array wasn't an option back then, otherwise I'd have
used it). Using a vector doesn't only incur the unnecessary (but, at least
in isolation, unnoticeable) overhead of the heap allocation but makes the
code less clear IMO, as you'd expect this vector to be appended to,
somewhere, whereas we never do this.

 Considering the following commit, I think the chances of the change I
propose are slim, but if the vector remains, I'd at least add a comment
explaining that the size of this vector never changes and we use it here
instead of an array just because... I don't actually know why, but you
should be able to fill in the dots.

 Regards,
VZ