11

So I noticed it's possible to avoid putting private functions in headers by doing something like this:

// In file pred_list.h:
    class PredicateList
    {
        int somePrivateField;
        friend class PredicateList_HelperFunctions;
    public:
        bool match();
    } 

// In file pred_list.cpp:
    class PredicateList_HelperFunctions
    {
        static bool fullMatch(PredicateList& p)
        {
            return p.somePrivateField == 5; // or whatever
        }
    }

    bool PredicateList::match()
    {
        return PredicateList_HelperFunctions::fullMatch(*this);
    }

The private function is never declared in the header, and consumers of the class which import the header don't ever need to know it exists. This is required if the helper function is a template (the alternative is putting the full code in the header), which is how I "discovered" this. Another nice upside of not needing to recompile every file that includes the header if you add/remove/modify a private member function. All the private functions are in the .cpp file.

So...

  1. Is this a well-known design pattern that there's a name for?
  2. To me (coming from a Java/C# background and learning C++ on my own time), this seems like a very good thing, since the header is defining an interface, while the .cpp is defining an implementation (and the improved compile time is a nice bonus). However, it also smells like it's abusing a language feature not intended to be be used that way. So, which is it? Is this something you would frown on seeing in a professional C++ project?
  3. Any pitfalls I'm not thinking of?

I'm aware of Pimpl, which is a much more robust way of hiding the implementation at the library edge. This is more for use with internal classes, where Pimpl would cause performance issues, or not work because the class needs to be treated as a value.


EDIT 2: Dragon Energy's excellent answer below suggested the following solution, which doesn't use the friend keyword at all:

// In file pred_list.h:
    class PredicateList
    {
        int somePrivateField;
        class Private;
    public:
        bool match();
    } 

// In file pred_list.cpp:
    class PredicateList::Private
    {
    public:
        static bool fullMatch(PredicateList& p)
        {
            return p.somePrivateField == 5; // or whatever
        }
    }

    bool PredicateList::match()
    {
        return PredicateList::Private::fullMatch(*this);
    }

This avoids the shock factor of friend (which seems to have been demonized like goto) while still maintaining the same principle of separation.

  • 2
    "A consumer could define their own class PredicateList_HelperFunctions, and let them access the private fields." Wouldn't that be an ODR violation? Both you and the consumer would have to be defining the same class. If those definitions are not equal, then the code is ill-formed. – Nicol Bolas Dec 9 '18 at 3:11
12

It's a bit esoteric to say the least as you already recognized which might have me scratching my head for a moment when I first start encountering your code wondering what you're doing and where these helper classes are implemented until I start to pick up your style/habits (at which point I might get totally used to it).

I do like that you're reducing the amount of information in the headers. Especially in very large codebases, that can have practical effects to reduce compile-time dependencies and ultimately build times.

My gut reaction though is that if you feel a need to hide implementation details this way, to favor parameter passing to free-standing functions with internal linkage in the source file. Usually you can implement utility functions (or whole classes) useful for implementing a particular class without having access to all the internals of the class and instead just pass in the relevant ones from the implementation of a method to the function (or constructor). And naturally that has the bonus of reducing coupling between your class and the "helpers". It also has a tendency to generalize what might have otherwise been "helpers" further if you find that they're starting to serve a more generalized purpose applicable to more than one class implementation.

I also sometimes cringe a bit when I see lots of "helpers" in code. It's not always true but sometimes they can be symptomatic of a developer who is just decomposing functions willy-nilly to eliminate code duplication with huge blobs of data being passed around to functions with barely comprehensible names/purposes beyond the fact that they reduce the amount of code required to implement some other functions. Just a little teeny bit more thought upfront can sometimes lead to much greater clarity in terms of how the implementation of a class is decomposed into further functions, and favoring passing specific parameters to passing whole instances of your object around with full access to internals can help promote that style of design thought. I'm not suggesting you're doing that, of course (I have no idea), but maybe to watch out for that tendency when we get too happy with the "helpers".

If that becomes unwieldy, I'd consider a second, more idiomatic solution which is the pimpl (I realize you mentioned issues with it but I think you can generalize a solution to avoid those with minimal effort). That can move a whole lot of information your class needs to be implemented including its private data away from the header wholesale. The performance issues of the pimpl can largely be mitigated with a dirt-cheap constant-time allocator* like a free list while preserving value semantics without having to implement full-blown user-defined copy ctor.

  • For the performance aspect the pimpl does introduce a pointer overhead at the very least, but I think the cases have to be pretty serious where the poses a practical concern. If the spatial locality isn't degraded significantly through the allocator, then your tight loops iterating over the object (which should generally be homogeneous if performance is that much of a concern) will still tend to minimize cache misses in practice provided you use something like a free list to allocate the pimpl, putting the fields of the class into largely contiguous memory blocks.

Personally only after exhausting those possibilities would I consider something like this. I do think it's a decent idea if the alternative is like more private methods exposed to the header with perhaps only the esoteric nature of it being the practical concern.

An Alternative

One alternative that popped into my head just now that largely accomplishes your same purposes absent friends is like this:

struct PredicateListData
{
     int somePrivateField;
};

class PredicateList
{
    PredicateListData data;
public:
    bool match() const;
};

// In source file:
static bool fullMatch(const PredicateListData& p)
{
     // Can access p.somePrivateField here.
}

bool PredicateList::match() const
{
     return fullMatch(data);
}

Now that might seem like a very moot difference and I'd still call it a "helper" (in a possibly derogatory sense since we're still passing the entire internal state of the class to the function whether it needs it all or not) except it does avoid the "shock" factor of encountering friend. In general friend looks a bit scary to see frequently absent further inspection, since it says that your class internals are accessible elsewhere (which carries the implication that it might be incapable of maintaining its own invariants). With the way you are using friend it becomes rather moot if people are aware of the practice since the friend is just residing in the same source file helping to implement the private functionality of the class, but the above accomplishes much the same effect at least with the one possibly arguable benefit that it doesn't involve any friends which avoids that whole kind ("Oh shoot, this class has a friend. Where else does its privates get accessed/mutated?"). Whereas the immediately above version immediately communicates that there's no way for the privates to be accessed/mutated outside of anything done in the implementation of PredicateList.

That is perhaps moving towards somewhat dogmatic territories with this level of nuance since anyone can quickly figure out if you uniformly name things *Helper* and put them all in the same source file that it's all kind of bundled together as part of the private implementation of a class. But if we get nit-picky then maybe the immediately above style won't cause as much of a knee-jerk reaction at a glance absent the friend keyword which tends to look a little bit scary.

For the other questions:

A consumer could define their own class PredicateList_HelperFunctions, and let them access the private fields. While I don't see this as a huge issue (if you really wanted at those private fields you could do some casting), maybe it would encourage consumers to use it that way?

That might be a possibility across API boundaries where the client could define a second class with the same name and gain access to the internals that way without linkage errors. Then again I'm largely a C coder working in graphics where safety concerns at this level of "what if" are very low on the priority list, so concerns like these are just ones I tend to wave my hands at and do a dance and try to pretend like they don't exist. :-D If you're working in a domain where concerns like these are rather serious though, I think that's a decent consideration to make.

The above alternative proposal also avoids suffering this issue. If you still want to stick to using friend though, you can also avoid that issue by making the helper a private nested class.

class PredicateList
{
    ...

    // Declare nested class.
    class Helper;

    // Make it a friend.
    friend class Helper;

public:
    ...
};

// In source file:
class PredicateList::Helper
{
    ...
};

Is this a well-known design pattern that there's a name for?

None to my knowledge. I kind of doubt there would be one since it's really getting into the minutia of implementation details and style.

"Helper Hell"

I got a request for further clarification on the point about how I sometimes cringe when I see implementations with lots of "helper" code, and that might be slightly controversial with some but it is actually factual as I really did cringe when I was debugging some of my colleagues' implementation of a class only to find loads of "helpers". :-D And I wasn't the only on the team scratching my head trying to figure out what all these helpers are supposed to do exactly. I also don't want to come off dogmatic like "Thou shalt not use helpers," but I would make a tiny suggestion that it might help to think about how to implement things absent of them when practical.

Aren't all private member functions helper functions by definition?

And yes, I am including private methods. If I see a class with like a straightforward public interface but like an endless set of private methods which are somewhat ill-defined in purpose like find_impl or find_detail or find_helper, then I also cringe in a similar way.

What I'm suggesting as an alternative is nonmember nonfriend functions with internal linkage (declared static or inside an anonymous namespace) to help implement your class with at least a more generalized purpose than "a function which helps implement others". And I can cite Herb Sutter from C++ 'Coding Standards' here for why that can be preferable from a general SE standpoint:

Avoid membership fees: Where possible, prefer making functions nonmember nonfriends. [...] Nonmember nonfriend functions improve encapsulation by minimizing dependencies: The body of the function cannot come to depend on the nonpublic members of the class (see Item 11). They also break apart monolithic classes to liberate separable functionality, further reducing coupling (see Item 33).

You can also understand the "membership fees" he talks about to some degree in terms of the basic principle of narrowing variable scope. If you imagine, as the most extreme example, a God object which has all the code required for your entire program to run, then favoring "helpers" of this sort (functions, whether member functions or friends) which can access all the internals (privates) of a class basically render those variables no less problematic than global variables. You have all the difficulties of managing state correctly and thread safety and maintaining invariants that you would get with global variables in this most extreme example. And of course most real examples are hopefully not anywhere close to this extreme, but information hiding is only as useful as it is limiting the scope of the information being accessed.

Now Sutter already gives a nice explanation here but I'd also add further that the decoupling tends to promote like a psychological improvement (at least if your brain works like mine) in terms of how you design functions. When you start designing functions that can't access everything in the class except only the relevant parameters you pass it or, if you pass the instance of the class as a parameter, only its public members, it tends to promote a design mindset that favors functions which have a clearer purpose, on top of the decoupling and promoting improved encapsulation, than what you might otherwise be tempted to design if you could just access everything.

If we go back to the extremities then a codebase riddled with global variables doesn't exactly tempt developers to design functions in a way that's clear and generalized in purpose. Very quickly the more information you can access in a function, the more many of us mortals face the temptation to degeneralize it and reduce its clarity in favor of accessing all this extra information that we have instead of accepting more specific and relevant parameters to that function to narrow its access to state and widen its applicability and improve its clarity of intentions. That applies (although generally to some lesser degree) with member functions or friends.

  • 1
    Thanks for the input! I don't totally understand where you're coming from with this part, though: "I also sometimes cringe a bit when I see lots of "helpers" in code." -- Aren't all private member functions helper functions by definition? This seems to take issue with private member functions in general. – Robert Fraser Dec 8 '18 at 17:44
  • 1
    Ah, the inner class doesn't need "friend" at all, so doing it that way totally avoids the "friend" keyword – Robert Fraser Dec 8 '18 at 17:49
  • "Aren't all private member functions helper functions by definition? This seems to take issue with private member functions in general." It's not the biggest thing. I used to think it was a practical necessity that for a non-trivial class implementation, you either have a number of private functions or helpers with access to all the class members at once. But I looked at the style of some of the greats like Linus Torvalds, John Carmack, and although the former codes in C, when he codes the analogical equivalent of an object, he manages to generally code it with neither along with Carmack. – Dragon Energy Dec 8 '18 at 22:33
  • And naturally I think helpers in the source file are preferable to some massive header that includes way more external headers than necessary because it used a lot of private functions to help implement the class. But after studying the style of those above and others, I realized it's often possible to write functions that are a bit more generalized than the types that need access to all the internal members of a class even to just implement one class, and the thought upfront to name the function well and pass it the specific members it needs to work often does end up saving more time [...] – Dragon Energy Dec 8 '18 at 22:36
  • [...] than it takes, yielding a clearer implementation overall that's easier to manipulate later on. It's like instead of writing a "helper predicate" for "full match" which accesses everything in your PredicateList, often it might be feasible to just pass a member or two from the predicate list to a slightly more generalized function which doesn't need access to every private member of PredicateList, and often that will tend to also yield a clearer, more generalized name and purpose to that internal function as well as more opportunities for "hindsight code reuse". – Dragon Energy Dec 8 '18 at 22:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.