Search code examples
c++pointersreferencereturn-by-value

How should I return an object from a function?


Consider the following scenario: There is a class CDriver that is in charge of enumerating all the attached output devices (represented by the COutput class). The code for that may look something like this:

class COutput
{
    // COutput stuff
};

class CDriver
{
public:
    CDriver();  // enumerate outputs and store in m_outputs

    // some other methods

private:
    std::vector<COutput> m_outputs;
};

Now CDriver should be able to grant the user access to the enumerated COutputs.

The first method of achieving this is to return a pointer:

const COutput* GetOutput(unsigned int idx) const 
{ 
    return idx < m_outputs.size() ? &m_outputs[idx] : nullptr; 
}

The way I see it, this method presents the problem that if the pointer is stored by the user and it persists after the CDriver object has been destroyed, it is now a dangling pointer. This is due to the fact that the pointee (COutput object) has been destroyed during the destructor of the CDriver object.

The second way of doing this would be to return by reference:

const COutput& GetOutput(unsigned int idx) const 
{ 
    return idx < m_outputs.size() ? &m_outputs[idx] : m_invalidOutput; 
}

Here the same problems apply as in the approach with pointers. Furthermore it has the additional caveat that no real invalid object can be returned. If a nullptr is returned as a return pointer, it is obvious that it is "invalid". There is, however, no equivalent to nullptr when it comes to references.

Moving on to approach number three. Return by value.

COutput GetOutput(unsigned int idx) const 
{ 
    return idx < m_outputs.size() ? &m_outputs[idx] : m_invalidOutput; 
}

Here, the user doesn't have to worry about the lifetime of the returned object. However, the COutput object has to be copied and there is, similary to the reference approach, no intuitive way to check for errors.

I could go on...

For example, the COutput objects could be allocated on the heap and stored in std::shared_ptrs and returned as such. This, however would make the code very verbose.

Is there any way to solve this problem intuitively and without introducing unnecessary code verbosity?


Solution

  • Let me start off by saying, you should absolutely not start messing around with shared_ptr to solve this problem. Just don't do it. You have a few different options here that are reasonable.

    First, you can simply return by value. If COutput is small, this is a good way to go. To deal with an out of bounds index, you have two options. One is throw an exception. Works well and is easy. This is what I would recommend here most likely. Make sure to have a size() member that the user can call to get the size, so they can avoid paying the cost of throwing, if that is too expensive for them. You can also return an optional. This is in the standard library as of 17, in boost prior, and there are standalone implementations.

    Second, you can return by pointer/reference. Yes, it can dangle. But C++ does not claim to offer protection against this. Every single standard container features begin() and end() methods that return iterators can easily dangle as well. Expecting clients to avoid these pitfalls is not unreasonable in C++ (you should of course document them though).

    Third, you can do inversion of control: instead of giving the user an object to operate on, you instead make the user pass in the action they want to take. In other words:

    template <class F>
    auto apply(std::size_t idx, F f) const 
    {
        if (idx >= m_outputs.size())
        {
            throw std::out_of_range("index is out of range");
        }
    
        return f(m_outputs[idx]); 
    }
    

    Usage:

    CDriver x;
    x.apply(3, [] (const COutput& o) {
        o.do_something();
    });
    

    The user needs to work quite a bit harder to make something dangle in this case (though it's still possible) since they aren't handed a pointer/reference, and you don't have to make a copy either.

    You can of course change apply in many ways; e.g. not return back from the functional call but instead return true/false to indicate whether the index was in range instead of throwing. The basic idea is the same. Note that this approach would have to be modified to be used in conjunction with virtual functions, which would make it less desirable. So if you are thinking of polymorphism for CDriver, you should consider that.