
Nevin ":-]" Liber wrote:
2008/5/16 Noah Roberts
: Nevin ":-]" Liber wrote:
2008/5/16 Noah Roberts
mailto:roberts.noah@gmail.com>: 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.