Strange Code: Returning a boost::scoped_ptr by Reference?

This topic is 3691 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

Recommended Posts

I was just going through some of the code at work. I'm happy I noticed I got a few of my coworkers using the boost smart pointers. However, when I ran into code similar to this, I was quite concerned:
class ClassA
{
public:
...
boost::scoped_ptr<ComponentA>& getComponent()
{
boost::scoped_ptr<ComponentA>& pComponent = createComponentAIfItDoesntExist(); // returns:  boost::scoped_ptr<ComponentA>&

return pComponent;
}

private:
boost::scoped_ptr<ComponentA> pComponent_;
};


I'm guessing the programmer that was doing this realized that scoped_ptrs<> can't be copied. So, instead of just returning a reference to the actual object held, he passes the reference to the actual scoped_ptr<> all over the place. Maybe he thought a scoped_ptr<> is expensive to dereference? I don't know. Maybe I'm nitpicking, but I think that sort of code is extremely strange.

Share on other sites
This looks like a weird use of the scoped_ptr class. scoped_ptr is just like std::auto_ptr from the STL but forbids copy construction for more safety.

Normally shared_ptr is the class one should use when the objects share ownership. scoped_ptr is normally used only within functions to ensure exception safety and automatic object deallocation. I rarely need this.

The scoped_ptr in the private section of your code snipped indicates the pointed to object only has a single owner that will be an instance of class A. If pComponent_ is never changed it could also be a simple member variable. Otherwise the use is ok, just like with auto_ptr.

Depending on what the guy that wrote the code intended to do the code might be ok.
But probably he actually just wanted to return a reference to the contained object and not to the while scoped_ptr.

Share on other sites
That's lazy creation, equivalent to:
class ClassA{public:    ComponentA * getComponent()    {      if (pComponent_ == NULL) pComponent_ = new ComponentA();      return pComponent_;    }    // or alternatively    Component & getComponentByRef()    {      if (pComponent_ == NULL) pComponent_ = new ComponentA();      return *pComponent_;          }private:    ComponentA * pComponent_;};

The code itself may seem convoluted, but it's correct. References and scoped_ptr however obstruct the ownership.

Whether this is useful or recommended pattern is something else, but it's not incorrect.

I'm not really sure if scoped_ptr is required here, reference should be enough for most purposes.

Share on other sites
The scoped pointer means that you no longer need to write a destructor, nor derive from boost::noncopyable. However, I would agree that is does not need to leak into the interface.

Share on other sites
Quote:
 Original post by rip-offThe scoped pointer means that you no longer need to write a destructor, nor derive from boost::noncopyable. However, I would agree that is does not need to leak into the interface.

Those are some side-effects, yes. I totally understand the use of scoped_ptr<>. But it being in the interface really makes no sense, since any client will just dereference it anyway to do anything with it. That was all. Just a tad convoluted and giving me the impression that my coworker didn't fully understand what scoped_ptr<> was for.

1. 1
2. 2
frob
16
3. 3
4. 4
5. 5

• 20
• 13
• 14
• 76
• 22
• Forum Statistics

• Total Topics
632145
• Total Posts
3004381

×