Search code examples
c++pointersshared-ptr

Return new instance or itself as std::shared_ptr rather than raw pointer


I implemented CSS-like selector system that can be used in some object tree. My selectors are not perfectly optimal at the time of creation. Sometimes, it is possible to replace more complex selector with simple one, which is a different class type.

I added this method to my selector interface:

virtual std::shared_ptr<Selector> optimize(std::shared_ptr<Selector> target) const = 0;

But there's a problem - if the selector cannot be optimized into some simpler class, it should return itself. For example, I have a selector that maintains list or items. If there is only one item, this selector is redundant and should return simpler selector, otherwise, it should return self.

std::shared_ptr<Selector> CSSChainedSelector::optimize() const
{
    if(sub_selectors.size()==1) {
        return sub_selectors[0]->optimize();
    }
    else if(sub_selectors.empty()) {
        return std::shared_ptr<Selector>(new InvalidSelector());
    }
    else {
        //THIS WILL CAUSE MEMORY ERROR IN THE FUTURE!!!
        return std::shared_ptr<Selector>(this);
    }
}

You mustn't create shared_ptr from this though. If you do, you cause the object to be deleted twice. Shared pointer must only be created from new instances. Knowing this, I had to change my design, which is now kinda crazy:

std::shared_ptr<Selector> CSSChainedSelector::optimize(std::shared_ptr<Selector> target) const
{
    if(target.get() != this)
        return target;
    // this is probably redundant
    std::shared_ptr<CSSChainedSelector> self = std::dynamic_pointer_cast<CSSChainedSelector>(target);
    if(self) {
        if(chain_.length() == 1) {
            return chain_[0]->optimize(chain_[0]);
        }
    }
    else
        return target;
    return target;
}

This requires me to call the method like this:

std::shared_ptr<Selector> selector(new CSSChainedSelector);
// parsing happens
   ... some parsing ...
selector = selector->optimize(selector);

This makes some sense since optimize method can only be called if you see the class from the outside (it doesn't change the class). Optimizations that change the class can be done during parsing.

But is there a way to safely return shared pointer on class's own instance?

Note: The optimization described above looks like microoptimalization, because I have strongly simplified the problem. Please focus on the topic of "returning own instance as shared pointer".


Solution

  • #include <memory>
    
    struct selector : std::enable_shared_from_this<selector>
    {
    
      std::shared_ptr<selector> optimise()
      {
    
        // the case where we can't optimise, we return ourselves.
        return this->shared_from_this();
      }
    };
    

    update: forgive me, your optimise function is const. If you're returning a shared_ptr to self it must therefore be a shared_ptr to const:

    #include <memory>
    
    struct selector : std::enable_shared_from_this<selector>
    {
    
      std::shared_ptr<const selector> optimise() const
      {
    
        // the case where we can't optimise, we return ourselves.
        return this->shared_from_this();
      }
    };
    

    unless you want to return a copy, but I think that probably defeats the object of the exercise.