Re: [lmi] [lmi-commits] master 43dcd1b 5/6: Make all table_column_info data members private and non-const

[Vadim Zeitlin]

On Fri, 24 Aug 2018 14:30:50 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2018-08-24 00:07, Vadim Zeitlin wrote:
GC> > On Thu, 23 Aug 2018 19:59:30 -0400 (EDT) Greg Chicares <address@hidden> 
wrote:
GC> > 
GC> > GC> branch: master
GC> > GC> commit 43dcd1b788a27f55c67de39c07e62d6fce781ca2
GC> > GC> Author: Gregory W. Chicares <address@hidden>
GC> > GC> Commit: Gregory W. Chicares <address@hidden>
GC> > GC> 
GC> > GC>     Make all table_column_info data members private and non-const
GC> > GC>     
GC> > GC>     It is impossible to change members' values after construction, so 
there
GC> > GC>     is no longer any need for them to be const; and their constness 
was a
GC> > GC>     problem because it made the class nonassignable.
GC> > 
GC> >  I don't really understand why the fields shouldn't be const if it's
GC> > impossible to change them: IMHO this is exactly the reason to make them
GC> > const.
GC> 
GC> These are all private members with public const accessors.

 If you can forgive my nitpicking, I don't really like this on its own
neither: what's the benefit of encapsulation if we provide trivial
accessors for the fields anyhow? I think just exposing the members as they
are is good enough when they're const and is simpler and, in some sense,
more honest than creating an illusion of encapsulation when there is none.

GC> Given that, what is the benefit of making them (or most of them) const?

 Sorry if it's a trick question, but my naive answer is that, of course,
making them "const" ensures they're not mutated in the class code itself
and not only from outside it. And if they really don't have to be changed,
this can only be a good thing.

GC> So I want col_width_ to have the same constness and accessibility
GC> as its siblings, and you want its siblings to be const members.
GC> Do you like the following patch, which I believe meets both those
GC> criteria simultaneously?

 Yes, thank you, I do agree that this is better. But I don't understand why
do we need to use resized_columns and swap(), what exactly prevents us from
emplacing the objects directly into all_columns_?

 I.e. my ideal version would avoid the temporary vector and my most ideal
version would also just make the fields public and drop the accessors.

 Thanks for addressing this,
VZ