pImpl idiom methods

Question

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?

if you were using pimpl properly, the you'd simply call `pImpl->Bar()` in `Foo::Bar(...)` — Nim, Mar 06 '15 at 21:48
That is what I am asking? So should this method be in the pImpl? Or am I misunderstanding something. — marsh, Mar 06 '15 at 21:48
As I said, if you were doing it properly, yes it would. Your `Foo` class should only hold one member `pImpl`, and all public functions in `Foo` should call the methods in the implementation... — Nim, Mar 06 '15 at 21:50
My thoughts are if you are using malloc and memcpy in a c++ program you probably should not be using pimpl. — drescherjm, Mar 06 '15 at 21:53
If you hate pImpl->bar so much, try wrapping it in a private method and call that wrapper everywhere. — user3528438, Mar 06 '15 at 22:12
@drescherjm My thoughts are if you are using malloc and memcpy in a c++ program you probably should not be using c++ — M.M, Mar 06 '15 at 22:59
@marsh can you clarify your question. Is `Foo` your public class? If so then "it seems a bit ugly to put pImpl->member every few words" doesn't make sense because your code sample does not actually put `pImpl->` anywhere so I don't see how it is working - `m_data` should be in the pImpl class. — M.M, Mar 06 '15 at 23:10
You should not need to define any public functions in your PIMPL class: https://stackoverflow.com/questions/24635255/is-it-possible-to-write-an-agile-pimpl-in-c/24638702#24638702 — Galik, Mar 06 '15 at 23:24

score 2 · Accepted Answer · answered Mar 06 '15 at 23:17

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.

pImpl idiom methods

1 Answers1