Re: [Boost-users] shared_ptr<> null deleter

From: Noah Roberts (roberts.noah_at_[hidden])
Date: 2008-05-17 00:17:01

Nevin ":-]" Liber wrote:
> 2008/5/16 Noah Roberts <roberts.noah_at_[hidden]>:
>> Nevin ":-]" Liber wrote:
>>> 2008/5/16 Noah Roberts <roberts.noah_at_[hidden]
>>> <mailto:roberts.noah_at_[hidden]>>:
>>>
>>>     I don't like this idea.  You are creating a dependency on the fact that
>>>     the called function will NOT keep a copy
> 
> Suppose I had the function:

More to the point.
> 
> void foo(int const* p)
> {
>     if (p) std::cout << *p << std::endl;
> }

I don't think foo in this case stands as an exemplary of good design
choice.  It should look more like so:

void foo()
{
}

void foo(int i)
{
  std::cout << i << std::endl;
}

Overrides like this are one of the great features of C++.

You might argue that foo looks more like:

void foo(int * p = 0) // I see this a lot.
{
  // ...bunch of stuff...
  *p = result of calculations
  // ... some cleanup maybe
}

Again, split it into two:

int bunch_of_stuff() { ... }
void cleanup() { ... }

void foo() { bunch_of_stuff(); cleanup(); }
void foo(int & p) { p = bunch_of_stuff(); cleanup(); }

You'll notice that any time you could have the =0 foo you can use these
two without any difference in calling semantics.  In both you either
call with an argument or don't.  Only one case, and one you should
never, ever do, is if you're passing around null pointers without caring.

The point is that you've got a function that behaves differently based
on a state variable being passed in.  That may be a necessary evil but
does not make for a good argument about best practice.  I've happily
kept my use of raw pointers to a bare minimum for quite a while now.
Everyone on my team is more productive because of it.

The difference here between using a pointer and splitting into two
functions that use shared code but behave differently is really much
more than just the difference between accepting pointer and reference as
far as maintaining goes, but there's a lot there so I'm going to pass on
that.  What is important per this discussion is that since you are
binding to the idea that you will not copy this data and hold it, make
that as obvious as possible.  Pointers are notoriously vague in their
meaning and one always asks, "Is this function going to copy and hold
this pointer?"

Bind to as little as possible.  Make clear any ambiguity.

You've come up with a plan that works for you, but then somebody like me
might come along and screw it all up.  Who's fault is it, the one that
didn't intuitively grasp the meaning of accepting raw pointers or the
one that didn't clarify as much as possible by not doing so to begin with?

If you really do need to do the whole if(p) thing, which is really
smelly, then you might consider using a reference wrapper that is
capable of being null.  Make intentions clear, "this will not be
copied," and leave the possibility of having an = 0 default.

Pointers are necessary in some cases--you're not going to implement a
vector without them--but really, in my opinion, should be avoided like
the plague.  You might be smart enough to use them without screwing up,
I have to protect myself from myself.

Now, again, there are cases when best practices go out the window in
favor of other, more important criteria.  If performance is really,
really an issue then wrapping up every pointer you see is not exactly
going to help you meet your criteria.  So you sacrifice clarity for
performance sometimes.  But when you need to do this you know
it...you've tested with a profiler and made your algorithm as efficient
as possible and know that this shared_ptr is really, really causing you
problems.