Re: [lmi] Non-null smart pointer

[Vadim Zeitlin]

On Tue, 16 Oct 2018 01:10:07 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2018-10-15 22:27, Vadim Zeitlin wrote:
GC> > On Mon, 15 Oct 2018 22:15:52 +0000 Greg Chicares <address@hidden> wrote:
GC> > 
GC> > GC> On 13/10/2018 22.54, Vadim Zeitlin wrote:
GC> > GC> > On Sat, 13 Oct 2018 22:08:48 +0000 Greg Chicares <address@hidden> 
wrote:
GC> > GC> > 
GC> > GC> > GC> Does lmi need exactly one kind of smart pointer, or more than 
one?
GC> > GC> > 
GC> > GC> >  Right now I can't point to any precise place where we need either
GC> > GC> > shared_ptr<> or scoped_ptr<> or even unique_ptr<> which can be 
null. But
GC> > GC> > this doesn't mean there any no such places.
GC> > GC> 
GC> > GC> We can't demonstrate that there are no such places now, or that none 
will
GC> > GC> be introduced in the future. That's a problem.
GC> > 
GC> >  Sorry, but why is this a problem? It would only become a problem if/when
GC> > we decided to get rid of shared_ptr<> and unique_ptr<> completely
GC> > (replacing the later with not_null_unique_ptr<>), but how is it a problem
GC> > right now?
GC> 
GC> We're actually talking about two different things.
GC> 
GC> You're saying that, of all the places where we use smart pointers, you
GC> don't immediately notice any where the value could appropriately be
GC> null. That's not a problem: any such situation will make itself manifest
GC> pretty quickly.
GC> 
GC> I'm saying that we don't know whether any of our smart pointers could
GC> potentially be null where null is inappropriate.

 But we know this. I.e. right now the factory functions return smart
pointers that could be null even though this would be totally
inappropriate.

GC> That, of course, is the problem that not_null_unique_ptr<> would solve

 Exactly. So I say let's solve it.

GC> (except where we really need not_null_shared_ptr<>, but I don't think
GC> we will need that).

 I'm really not sure about why/where does lmi need shared_ptr<> at all, so
I can't say. In theory, not_null_shared_ptr<> could be useful too.


GC> I've surveyed lmi's usage of smart pointers in general, and I think we
GC> need far fewer of them, and dramatically fewer shared_ptrs in particular.

 Undoubtedly. But this doesn't mean we don't need any of them.

GC> Might we indeed find that we almost never need smart pointers?

 I don't think so as this is basically equivalent to saying that we never
need heap allocation outside of standard containers. While possible, IME
this is rarely true for any non-trivial program.

GC> >  I think we should start by adding and using not_null_unique_ptr<> in the
GC> > places in which it is better suited than unique_ptr<> (and again, we
GC> > already know that this is a non-empty set). We know that it won't be able
GC> > to replace all instances where different other smart pointers are used, 
but
GC> > so what?
GC> 
GC> Even better than a smart pointer that's constrained to be non-null
GC> is an object that by its nature cannot possibly be null.

 Of course. And yes, using heap-allocated instead of stack-allocated
objects is a common mistake in C++. But it's not _always_ a mistake.

GC> > GC> That's a well-defined bucket, independent of lmi's present use of 
standard
GC> > GC> smart pointers. Is gui_ptr<> small enough to show its implementation 
here?
GC> > 
GC> >  It's trivial. I.e. basically it's just
GC> > 
GC> >   template <typename T> using gui_ptr = T*;
GC> > 
GC> > As I said, I'm not really sure if it's worth using.
GC> 
GC> Oh, I thought perhaps you had written it with a non-null constraint.

 It can't really be always non-null... E.g. a parent wxWindow* can be null
sometimes. But I agree that it could be a good idea to have
not_null_gui_ptr<> too (but this really starts being a mouthful).

GC> I've already done some work there for
GC>   group_quote_pdf_gen*.?pp
GC>   progress_meter*.?pp
GC> and it would probably be simpler if I could just commit some changes
GC> for your review.

 Sure, no problem.

GC> As for all the shared_ptr members of class BasicValues, I've managed
GC> (in changes that aren't ready to commit) to replace smart pointers
GC> with objects. But this leads to changes like:
GC> 
GC> diff --git a/basic_values.hpp b/basic_values.hpp
GC> +#include "database.hpp"
GC> ...
GC> -    std::shared_ptr<product_database>   Database_;
GC> +    product_database        const       database_;
GC> 
GC> diff --git a/basicvalues.cpp b/basicvalues.cpp
GC> -#include "database.hpp"
GC> ...
GC>  BasicValues::BasicValues(Input const& input)
GC>      :yare_input_              {input}
GC> +    ,policy_                  {}
GC> +    ,database_
GC> +        ("empty for now" // filename
GC> +        ,yare_input_.Gender
GC> +        ,yare_input_.UnderwritingClass
GC> +        ,yare_input_.Smoking
GC> +        ,yare_input_.IssueAge
GC> +        ,yare_input_.GroupUnderwritingType
GC> +        ,yare_input_.StateOfJurisdiction
GC> +        )
GC> 
GC> and I'm not quite convinced that's ideal. If holding subobjects
GC> directly as const data members, and initializing them in the
GC> ctor-initializer, is good in general,

 If we always need them and there is no benefit in delaying their
allocation and initialization, I would say that it is indeed good.

GC> then is it possible to have too much of this good thing? That would
GC> become one honking huge ctor-initializer.

 You don't have to do it directly in the initializer, you could have a
helper function returning this object instead, after possibly constructing
it piecewise. And this function would profit from NRVO, so there would be
no unnecessary copying.

GC> > but I still think it could be better to start
GC> > with a low-hanging not_null_unique_ptr<> fruit rather than trying to 
revise
GC> > all pointers used in lmi. This is a worthwhile task, of course, but it 
will
GC> > take a much longer time to complete and in the meanwhile we still will 
have
GC> > to use unique_ptr<> even for pointers that can't be null, which is a pity.
GC> 
GC> You're looking for low-hanging fruit, but I'm asking whether we
GC> should bulldoze the orchard.

 I prefer incremental progress to revolutions, as sometimes the former may
make the latter unnecessary.

 Regards,
VZ