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

[Vadim Zeitlin]

On Wed, 25 Apr 2018 21:58:15 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2018-04-25 17:26, Vadim Zeitlin wrote:
[...]
GC> > ? Although, to be honest, I'd really prefer to used std::array here 
because
GC> > the main point of using an array and not a vector here originally was to
GC> > show that this array is not resizable (this code predates the switch to
GC> > C++11 in lmi, so std::array wasn't an option back then, otherwise I'd have
GC> > used it). Using a vector doesn't only incur the unnecessary (but, at least
GC> > in isolation, unnoticeable) overhead of the heap allocation but makes the
GC> > code less clear IMO, as you'd expect this vector to be appended to,
GC> > somewhere, whereas we never do this.
GC> 
GC> True; but OTOH, IIRC, every other argument to do_output_values() and
GC> output_row() was of type std::vector, and I favor consistency. In
GC> 'ledger_pdf_generator_wx.cpp', why not change this line, e.g.:
GC>     std::vector<std::string> output_values(columns.size());
GC> to use an array as well? One container type should be used in all
GC> similar situations.

 Unfortunately std::array can't be used here because the number of columns
is not known at compile-time. I actually think it might be worth modifying
this code to be template-, rather than inheritance-, based in order to make
it known, but I still write code using OO techniques more naturally by
default and, after having written it, I couldn't convince myself that it
was worth changing it. I don't know if I was right or wrong about it.

GC> I have little appetite for reopening this refactoring exercise again,
GC> but if I had, this pair of lines from two similar files troubles me
GC> more than the possibility of resizing a vector (which I now wouldn't
GC> dream of doing in this code):
GC> 
GC>             table.output_row(&pos_y,   output_values);
GC>         table_gen.output_row(&pos_y, i.output_values);
GC> 
GC> and I had half a mind to s/\<table\>/table_gen/ because the latter
GC> name is greppable.

 Yes, I agree that "table_gen" is probably a better name. I'm afraid I was
just lazy here and used a shorter one.

GC> But what really bothers me is that I only half rewrote
GC> wx_table_generator::output_header(). I never would have imagined
GC> slicing N contiguous elements from a vector of M*N elements as
GC>   &vec.at(k); // for k in {1..N-1}
GC> because I've never seen '&' written before a std::vector variable;

 But it's not really the address of a vector, just an iterator pointing to
the given element. Anyhow, I admit that this code is not totally limpid and
I tried to explain the data structure used in the comment preceding it
because of that. I still don't see a better way to do it, however.

GC> It still feels utterly alien to me, but so do some of AlphaGo's moves.

 Strange, AlphaGo moves seem intuitively very appealing to me, this is why
it's such a pleasure to see it crashing Stockfish with its inhuman play
style. It's just that when I play against Stockfish using my intuition, I
usually lose in 20 moves or less (if I try to actually compute the
consequences of my moves, I can make it last a bit longer, but the endgame
against it is still hopeless).

 Regards,
VZ