Putting all methods in class definition

Question

When I use the pimpl idiom, is it a good idea to put all the methods definitions inside the class definition? For example:

// in A.h

class A {
   class impl;
   boost::scoped_ptr<impl> pimpl;
public:
   A();
   int foo();
}

// in A.cpp

class A::impl {
   // method defined in class
   int foo() {
       return 42;
   }

   // as opposed to only declaring the method, and defining elsewhere:
   float bar();
};

A::A() : pimpl(new impl) { }
int A::foo() {
   return pimpl->foo();
}

As far as I know, the only problems with putting a method definition inside a class definition is that (1) the implementation is visible in files that include the class definition, and (2) the compiler may make the method inline.

These are not problems in this case since the class is defined in a private file, and inlining has no effect since the methods are called in only one place.

The advantage of putting the definition inside the class is that you don't have to repeat the method signature.

So, is this OK? Are there any other issues to be aware of?

Answer 1

I think you answered your own question : both solutions are equivalent.

However, I wouldn't be so sure that 'inlining has no effect since the methods are called in only one place' : an additional call could exists when the functions are not inlined. But chances are that the compiler is smart enough to optimize them away from the one-line forwarding calls in the outer class.

In the end, I believe it's just a matter of taste.

Answer 2

Advantages:

• all code of the class is localized

Disadvantages:

• for larger classes: when scrolling is needed, it becomes more difficult to know to which class the function belongs.
• dependencies are more easily solved when functions reside after all class declarations. Otherwise, it might be needed that some class declarations are moved after others and some functions still have to be moved after the class declaration when there are mutual dependency of internal classes.

Answer 3

Usually I don't add methods to the Impl inner class, but I can't see any issue if you define the methods inline. It seems to me much more readable than having seperate declaration and definition.

Answer 4

Whether the compiler inlines the methods depends on the compiler and the passed parameters.

In the case of the pimpl idiom, I don't think it matters whether the methods are defined within the Imp's body or not. I personally like them defined outside, because it is easy to see what is really important (like member variables and list of methods).