3

Consider these two classes that employ the Pimpl idiom:

ClassA: Pimpl class forward declaration and variable declaration on separate lines

ClassA.h:

#include <memory>

class ClassA {
public:
    ClassA();
    ~ClassA();
    void SetValue( int value );
    int GetValue() const;

private:

    class ClassA_Impl;
    // ^^^^^^^^^^^^^^ class forward declaration on its own line

    std::unique_ptr<ClassA_Impl> m_pImpl;
    // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ variable declaration on its own line

    //EDIT:
    //Even if we use a raw pointer instead of a smart pointer,
    //i.e. instead of declaring the smart pointer above, if we declare:
    ClassA_Impl *m_pImpl;
    //the situation in the *.cpp files and my questions (below) are the same.


};

ClassA.cpp:

#include "ClassA.h"

class ClassA::ClassA_Impl {
public:
    void SetValue( int value );
    int GetValue() const;
private:
    int value_;   
};

// Private class implementation
void
ClassA::ClassA_Impl::SetValue( int value ) {
    value_ = value;
}

int
ClassA::ClassA_Impl::GetValue() const {
    return value_;
}

// ClassA implementation
ClassA::ClassA() : m_pImpl( new ClassA_Impl() ) {}

ClassA::~ClassA() {}

void
ClassA::SetValue( int value ) {
    m_pImpl->SetValue( value );
}

int
ClassA::GetValue() const {
    return m_pImpl->GetValue();
}

ClassB: Pimpl class forward declaration and variable declaration on one line

ClassB.h:

#include <memory>

class ClassB {
public:
    ClassB();
    ~ClassB();
    void SetValue( int value );
    int GetValue() const;

    private:
        std::unique_ptr<class ClassB_Impl> m_pImpl;
        //             ^^^^^^^^^^^^^^^^^^ class forward declaration
        //             combined with variable declaration on one line,
        //             in one shot.

        //EDIT:
        //Even if we use a raw pointer instead of a smart pointer,
        //i.e. instead of declaring the smart pointer above, if we declare:
        class ClassB_Impl *m_pImpl;
        //the situation in the *.cpp files and my questions (below) are the same.
};

ClassB.cpp:

#include "ClassB.h"

class ClassB_Impl {
public:
    void SetValue( int value );
    int GetValue() const;
private:
    int value_;
};

// Private class implementation
void
ClassB_Impl::SetValue( int value ) {
    value_ = value;
}

int
ClassB_Impl::GetValue() const {
    return value_;
}

// ClassB implementation
ClassB::ClassB() : m_pImpl( new ClassB_Impl() ) {}

ClassB::~ClassB() {}

void
ClassB::SetValue( int nValue ) {
    m_pImpl->SetValue( nValue );
}

int
ClassB::GetValue() const {
    return m_pImpl->GetValue();
}

Questions:

  1. Why does combining the forward declaration and variable declaration on one line in ClassB.h require ClassB_Impl to be "unscoped" in the implementation of the private class in ClassB.cpp?

    i.e. in ClassA.cpp, private class method definitions begin with

    void ClassA::ClassA_Impl::foo() {...
    

    but in ClassB.cpp, private class method definitions begin with

    void ClassB_Impl::foo() {...
    
  2. What are the implications of each method? Which one is better?

  3. (Follow-up question in response to Galik's answer)

    When you combine forward declaration of a class and declaration of a variable of that class in one statement...

    //one-liner
    class ClassB_Impl *m_pImpl;
    

    ...what is this called? Is there a name for this kind of combined statement? And why exactly doesn't ClassB_Impl become an inner class of ClassB as a result of such a statement?

    Compare this to...

    //two-liner
    class ClassA_Impl;
    ClassA_Impl *m_pImpl;
    

    ...in which case ClassA_Impl does become an inner class of ClassA.

    Why does the one-liner put ClassB_Impl into the global namepace, while the two-liner puts ClassA_Impl into ClassA's namespace? Why are they different?

Quokka
  • 175
  • 1
  • 3
  • 10

1 Answers1

1

Why does combining the forward declaration and variable declaration on one line in ClassB.h require ClassB_Impl to be "unscoped" in the implementation of the private class in ClassB.cpp?

Because in the first example you declare ClassA_Impl as an inner class of ClassA.

When you declare ClassB_Impl it in the template parameter that is not part of ClassB.

What are the implications of each method? Which one is better?

This is a matter of opinion. Personally I think inner classes are messy and harder to work with for little reward.

My preferred method uses a separate interface class which helps to reduce the number of times you have to redeclare the interface.

See: Is it possible to write an agile Pimpl in c++?

Galik
  • 42,526
  • 3
  • 76
  • 100
  • "When you declare it in the template parameter that is not part of ClassB." - I edited the question to reflect that even if I use a raw pointer instead of a smart pointer (so that the private class is not declared in the template parameter), the situation is the same. So even with a raw pointer, does that mean a declaration like `class ClassB_Impl *m_pImpl` makes `ClassB_Impl` _not_ part of the class? Does that mean `ClassB_Impl` is available in the global namespace? – Quokka Feb 08 '16 at 08:35
  • Thinking about it more, I'm realizing that `ClassB_Impl` _is_ in the global namespace; it has to be, which is why in ClassB.cpp we can (in fact, we _must_) say `void ClassB_Impl::foo() {...` instead of `void ClassB::ClassB_Impl::foo() {...`. If this is in fact the case, then I think the answer to my Question 2 is that ClassA is better, because the internal Pimpl class stays hidden; unlike in ClassB, `ClassA_Impl` isn't available in the global namespace, which is what we'd prefer. Thoughts? Am I missing anything? – Quokka Feb 08 '16 at 08:59
  • @Quokka I think youve figured it all out now. The only way the class gets to be an *inner class* of `ClassB` is if you declare it as a class inside `ClassB`. This does not happen when you are simply declaring the type of one of `ClassB`'s member variables. – Galik Feb 08 '16 at 09:53