Re: [lmi] do_compute_column_widths_if_necessary()

[Vadim Zeitlin]

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

GC> On 2018-04-21 20:42, Vadim Zeitlin wrote:
GC> > On Sat, 21 Apr 2018 19:54:27 +0000 Greg Chicares <address@hidden> wrote:
GC> > 
GC> > GC> On 2018-04-21 13:08, Vadim Zeitlin wrote:
[...]
GC> > GC> Can you provide more context? I'm still having trouble understanding 
this.
GC> > GC> I understand that some group-quote columns are hidden (there's one 
large
GC> > GC> set of columns that are potentially shown, and some are suppressed at
GC> > GC> run time, e.g. if their contents are all zero). But AIUI either a 
column
GC> > GC> is hidden (and all its rows are hidden), or shown (and all its rows 
are
GC> > GC> shown). I'm supposing, perhaps incorrectly, that there comes a moment
GC> > GC> when we know which columns are hidden (at which point we might simply
GC> > GC> remove them from the vector of columns),
GC> > 
GC> >  There is no vector of columns.
GC> 
GC> On a literal level, there would seem to be, here:
GC> 
GC>     std::vector<column_info> columns_;

 Yes, sorry, I should have explained this better, but you've already
understood what I meant even though I didn't.

GC> Thus, data is passed not by column, but by row...

 This is exactly what I was trying to say, thank you for formulating it
clearly.


GC> > There is a vector of rows, where each row
GC> > is an array of values for the given row. We could, in principle, go 
through
GC> > this vector and filter out the values that need to be hidden from each row
GC> > (which would need to become a vector and not a simple array instead). But
GC> 
GC> I really don't like working with C-style arrays. And in fact there
GC> was only one of them, so I replaced it with a vector of strings.

 Sorry, but I think it's a bad idea as I tried to explain in my reply to
the corresponding commit message.

GC> I also changed the API that took "std::string const*", because it was
GC> needed only for that one C-style array--and all the vectors passed to
GC> it had call data() first, which feels like casting to void.

 Err, it's totally type-safe, so how could it be possibly similar to
casting to void? FWIW I disagree with this change too, there is no good
reason for wx_table_generator to impose the use of std::vector<> in its
API. Why shouldn't it be possible to use it with the elements stored in an
array (meaning std::array) or something else? There is nothing wrong with
the use of non-owning raw pointers in C++, IMO, and here it's used as a
form of a trivial iterator. If we want to be fancy, we could make this
function a template taking an arbitrary input iterator instead, but
requiring an std::vector is, again, just not justified from the API design
viewpoint IMHO.

 Regards,
VZ