lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] depr.impldec


From: Vadim Zeitlin
Subject: Re: [lmi] depr.impldec
Date: Fri, 15 Jul 2022 17:56:24 +0200

On Thu, 14 Jul 2022 21:23:36 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> On 7/14/22 13:56, Vadim Zeitlin wrote:
[...]
GC> >  In fact, maybe it's worth taking a step back and asking ourselves why
GC> > exactly do we want to prevent this from being done. I don't see anything
GC> > really wrong with having a vector<unique_ptr<datum_base>> for example, is
GC> > there?
GC> 
GC> I see no reason why that would not work. The question is whether the
GC> Principle of Least Privilege (PoLP) is sufficient reason to limit
GC> access as severely as is consistent with the design purpose.
GC> I tend to think that it is.

 This depends on the design goal of datum_base class and as you understand
it much better than me (if only, but hopefully not only, because I don't
understand it very well at all), I'll just trust you on this one.

 If you wanted -- but please don't feel obligated to do if you don't -- to
convince me of this using an argument rooted in something other than trust
in you, I'd like to understand who owns the datum_base objects and why is
unreasonable for their owner to store, and hence delete, them via the base
class pointer.

GC> > GC> For a base class like the erstwhile std::unary_function, a non-virtual
GC> > GC> dtor is just fine. For a base class like 'datum_base', a non-virtual
GC> > GC> dtor must change its nature to virtual along the inheritance chain,
GC> > GC> which, lacking any benefit, is just gratuitous weirdness.
GC> > 
GC> >  Maybe the real problem is that it becomes virtual, i.e. perhaps there is
GC> > something wrong with our inheritance hierarchy and we should avoid having 
a
GC> > class (datum_string) which is both a concrete class and a base class?
GC> 
GC> Meyers, MEC++ #33: "Make non-leaf classes abstract".

 Yes, I was indeed thinking along the same lines.

GC> My first reaction is that I would never assign a Lizard to a Chicken
GC> through a pointer to Animal, because I zealously avoid pointers.

 I think you know it just as well as I do but, just to be clear, there is
no difference between pointers and members in this context. There is a huge
difference between either of those and values and some people make an
excellent point that it's much better to use the former. If you haven't
seen it already, Sean Parent's talk about inheritance is one of the very
few videos (i.e. not article, but something that I'm ready to recommend
even in this form) about C++ that I believe are well worth watching:

        https://www.youtube.com/watch?v=bIhUE5uUFOA

GC> Except...
GC> 
GC> template<> struct reconstitutor<datum_base,Input>
GC> {
GC>     typedef datum_base DesiredType;
GC>     static DesiredType* reconstitute(any_member<Input>& m)
GC>         {
GC>         DesiredType* z = nullptr;
GC>         z = exact_cast<datum_string            >(m); if(z) return z;
GC>         z = reconstitutor<datum_sequence,Input>::reconstitute(m); if(z) 
return z;
GC>         z = reconstitutor<mc_enum_base  ,Input>::reconstitute(m); if(z) 
return z;
GC>         z = reconstitutor<tn_range_base ,Input>::reconstitute(m); if(z) 
return z;
GC>         return z;
GC>         }
GC> };
GC> 
GC> So my second reaction is that it looks like I might have taken
GC> adequate precautions, by (in effect) recapitulating the class
GC> hierarchy in reconstitute(). And it always works, or at least
GC> I'm unaware of any case where it doesn't...as long as no one
GC> foolishly changes the order above so that the exact_cast case
GC> isn't treated first.

 Yes, IMHO this function is not great.

GC> The question now is whether we should touch this and possibly
GC> break it, knowing that testing a deep change may require deep
GC> thought. It's been in production for decades and no one has
GC> ever reported a problem stemming from this OOP sin.
GC> 
GC> With a twinge of remorse, I must say I feel disinclined to
GC> pursue it.

 I understand this very well, it absolutely doesn't seem like an urgent
problem to fix. But if I were writing this code anew, I probably wouldn't
have done it like this.

GC> I'll commit it only if it gets a majority in favor, you and I
GC> being the eligible voters. Here's my case in favor:
GC> 
GC> - PoLP, but I already said that.

 I'm not exactly against this argument, because there is some merit in this
principle, of course, but I'm lukewarm because I think it carries its full
weight when designing widely used libraries. Here, where you control all
the uses of this class anyhow, the potential inconvenience of having to
make the dtor public if you do need it one day seems about as bad as the
potential gain from making it protected, so I just can't make myself care
much about it one way or the other.

GC> - "Make non-leaf classes abstract": if we were to follow through
GC> with that idea, wouldn't we make the special members of non-leaf
GC> classes (including datum_base) 'protected'? In that case, pushing
GC> that patch is at least a first, small, incremental step in the
GC> right direction.

 Yes, making special member functions of abstract classes protected would
be better.

GC> And here's my case against:
GC> 
GC> - This rule, for which I'll give a URL because it's an Americanism:
GC>   https://en.wikipedia.org/wiki/Pottery_Barn_rule

[Thanks, I knew the rule but I didn't know it was called that.]

GC> It may seem that making the base class's members 'protected'
GC> is perfectly safe, because the compiler would warn us of any
GC> actual usage that's broken thereby. We can detect any breakage
GC> at compile time...as long as we don't do anything that would
GC> move potential errors to run time...oops: 'reconstitutor'.
GC> The risk is not obviously less than refactoring datum_string
GC> into an abstract and a complete class. If we're going to run
GC> the risk, or rather pay the cost to satisfy ourselves that
GC> our change is correct, then we should do that refactoring;
GC> else, we should do nothing.
GC> 
GC> I'm not ready to cast a vote yet. What do you think?

 I'd prefer to abstain, if I have such a possibility. If I had absolutely
no other concerns and things to do, I would have done the refactoring, but
I'll be the first to admit that it doesn't gain us that much and, as any
non-trivial change, has a chance to break something, and so I'm not sure at
all about the risk/benefit balance here. You are better placed to estimate
it, so I think this decision should be yours.

 Sorry for lack of help,
VZ

Attachment: pgp6rIt12zAzc.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]