marsh
marsh

Reputation: 2740

pImpl idiom methods

I have implemented a basic pImpl set-up that is basically this example: Is the pImpl idiom really used in practice?.

Most pImple implementations I find online never show any method examples. The rule of thumb seems to be to hide all private members/methods in the pImple. The problem I am running into is I have a public method that looks sort of like this:

bool Foo::Bar(const void* data, size_t size)
{
    if( size > 0 )
    {
        if(data == nullptr)
            return false;

        size_t newSize = m_size + size;
        if ( newSize > m_capacity )
        {
            m_capacity = GROW_FACTOR*newSize;
            void* newMem = Malloc(m_capacity);

            if ( m_size > 0 )
                ::memcpy(newMem, m_data, m_size);

            if ( m_data )
                Free(m_data);

            m_data = (char*)newMem;
        }
        ::memcpy(m_data + m_size, data, size);
        m_size += size;
    }
    return true;
}

Should this be in the pImpl? If not it seems a bit ugly to put pImpl->member every few words. How do you go about this?

Upvotes: 0

Views: 378

Answers (1)

M.M
M.M

Reputation: 141648

Assuming I am interpreting your question correctly, i.e. that Foo is the public class and Bar is a public method. You have two options. Option A is:

void Foo::Bar()  // Foo is the public class
{
    pImpl->baz = 5;
}

and Option B is:

void Foo::Bar() { pImpl->Bar(); }

where the Impl class contains:

void Impl::Bar() { baz = 5; }

I'm not sure if there are any published guidelines on choosing between the two. In my own code I use Option A for simple methods and Option B once it starts getting to the stage that it seems like there are pimples everywhere. It is a balance between redundant code and extra argument copying, and pimple abundance.

However, as suggested in comments, perhaps it would be cleaner to use Option B for everything, or at least every non-trivial function.

Upvotes: 2

Related Questions