Storing a pass-by-reference parameter as a pointer - Bad practice?

Posted by Karl Nicoll on Programmers See other posts from Programmers or by Karl Nicoll
Published on 2013-10-22T23:03:36Z Indexed on 2013/10/23 4:09 UTC
Read the original article Hit count: 189

Filed under:
|

I recently came across the following pattern in an API I've been forced to use:

class SomeObject
{
public:
    // Constructor.
    SomeObject(bool copy = false);

    // Set a value.
    void SetValue(const ComplexType &value);

private:
    bool         m_copy;
    ComplexType *m_pComplexType;
    ComplexType  m_complexType;
};

// ------------------------------------------------------------

SomeObject::SomeObject(bool copy) :
  m_copy(copy)
{
}

// ------------------------------------------------------------

void SomeObject::SetValue(const ComplexType &value)
{
    if (m_copy)
        m_complexType.assign(value);
    else
        m_pComplexType = const_cast<ComplexType *>(&value);
}

The background behind this pattern is that it is used to hold data prior to it being encoded and sent to a TCP socket. The copy weirdness is designed to make the class SomeObject efficient by only holding a pointer to the object until it needs to be encoded, but also provide the option to copy values if the lifetime of the SomeObject exceeds the lifetime of a ComplexType.

However, consider the following:

SomeObject SomeFunction()
{
    ComplexType complexTypeInstance(1);  // Create an instance of ComplexType.

    SomeObject encodeHelper;
    encodeHelper.SetValue(complexTypeInstance); // Okay.

    return encodeHelper;

    // Uh oh! complexTypeInstance has been destroyed, and
    // now encoding will venture into the realm of undefined
    // behaviour!
}

I tripped over this because I used the default constructor, and this resulted in messages being encoded as blank (through a fluke of undefined behaviour). It took an absolute age to pinpoint the cause!

Anyway, is this a standard pattern for something like this? Are there any advantages to doing it this way vs overloading the SetValue method to accept a pointer that I'm missing?

Thanks!

© Programmers or respective owner

Related posts about c++

Related posts about pointers