Avoid includes required by private methods in header file

Question

A class concerned with a specific task relies on a certain 3rd party library to perform this task (lets say JSON serialization). The use of the library should be transparent for the client and none of the 3rd party code classes or data types are referenced in the public interface of the class.

Before I introduced a private method, I could just add the required #includes to the .cpp file. However, now that I declared a private method depending on the 3rd party code I have to pull up some #includes to the header file which in turn leads to inclusion of the respective headers in all other files including the header file of my class.

I was thinking about using functions instead of private methods so I would not have to declare the functions in the header. Of course I would have to pass references to the fields I'm using to these functions. Is this a reasonable way to work around this issue or are there best practices to achieve this kind of encapsulation while using private methods?

Answer 1

let say you have to add those private methods

• Json::Node AsJson() const;
• std::string Serialize(const Json::Node& root) const;
• Json::Node Unserialize(const std::string& document) const;
• void InitFrom(const Json::Node&);

FORWARD DECLARATION

Assuming Json is a namespace, in header you only need forward declaration

namespace Json
{
    class Node;
}

instead of

#include <3rdLibrary/Json.hpp>

if Json is a class and so Json::Node an inner class, you can't use forward declaration

FREE FUNCTIONS If the function doesn't require private access (and some other things as no virtual), you may create free functions (in unnamed namespace or static)

so nothing in header, and in cpp:

#include <3rdLibrary/JsonCpp.hpp>
namespace {
    Json::Node AsJson(const MyClass& myclass) {/**/}
    std::string Serialize(const MyClass& myclass, const Json::Node& root) {/**/}
    Json::Node Unserialize(const MyClass& myclass, const std::string& document) {/**/}
    void InitFrom(MyClass& myclass, const Json::Node&){/**/}
}
MyClass::foo()
{
    Serialize(*this, AsJson(*this));
    /**/
}

PIMPL IDIOM Pimpl idiom is an other alternative

// header.hpp

class MyClass
{
public:
    ~MyClass();
    MyClass(const MyClass&); // = delete ?
    MyClass& operator =(const MyClass&); // = delete ?
    MyClass(MyClass&&); // = delete ?
    MyClass& operator =(MyClass&&); // = delete ?

    // previous public methods.
private:
    struct Pimpl;
    std::unique_ptr<Pimpl> pimpl;
};

And in source

// MyClass.cpp

struct Pimpl
{
    // What you should have done in MyClass
};

MyClass::~MyClass() = default; // which destroys Pimpl and should know Pimpl definition
                               // that's why it is not in header

ReturnType MyClass::publicMethod(Args args) {return pimpl->publicMethod(args);} // Same for all public methods.

Note: it may be possible to hide with pimpl idiom only some part of implementation

INTERFACE

And finally, similarly to pimpl idiom, you may use interface

// IMyClass.hpp
class IMyClass
{
public:
    virtual ~IMyClass() = default;
    IMyClass(const IMyClass&); // = delete ?
    IMyClass& operator =(const IMyClass&); // = delete ?
    IMyClass(IMyClass&&); // = delete ?
    IMyClass& operator =(IMyClass&&); // = delete ?

    // previous public methods but virtual.
};

std::unique_ptr<IMyClass> makeMyClass(Args);

and implement MyClass normally (with override) (its header is only used by its cpp file)

and implement also

std::unique_ptr<IMyClass> makeMyClass(Args args) { return std::make_unique<MyClass>(args); }

Note: it may be possible to expose via interface only some part (to hide only some part of code).

Answer 2

Use a free (non-member) function in your implementation file, not a private method. Many consider private methods to be a code smell anyway.

This article by Scott Meyers (which @Jarod42 mentioned in a comment) is an excellent discussion of how and why to do it.

ETA: It's been awhile since I read Scott Meyers' article, and I forgot his focus was more on functions that might be part of the public interface to a class from the class creator's perspective. But think about his Wombat example from a class user's perspective. What you want to do is add to the interface of the class you're using, with out exposing that implementation detail to your class's users.

What I'm talking about are "hidden helpers", functions that don't need direct access to the internals of the class, but are used by the public functions of the class to do work, which is what it sounds like you've described. These are functions that are only visible inside the implementation file of a class, and to which a member function would pass copies or references to its data members.

A trivial example might be something like this:

namespace {

int frob( int coordinate )
{
    return coordinate * 3 + 9;
}

} // end anonymous namespace

void foo::shift_out()
{
    for ( auto & coord : m_coords )
    {
        coord = frob( coord );
    }
} 

So in this example, foo::shift_out() uses frob() to do the necessary work while frob() knows nothing about the internals of class foo and only the class foo implementation knows about the existence of frob().

As to private methods being a code smell, searching here on StackOverflow will give you a number of good discussions of the issue. It's seen as an indicator that the class may be trying to take on too much responsibility, which it should delegate to other classes which are then used to compose the original class.

Answer 3

PIMPL idiom will suit you best. In the main interface class just expose the public members and have a pointer to implementation.

class MyImpl;

class InterfaceClass
{
public:
    // Public methods go here.

private:
    MyImpl *pImpl;
};

Define the MyImpl class separately and it's header does not need to part of your interface files.