A Bad(?) Excuse for Private Inheritance


A few weeks ago I introudced private inheritance, and finished with a comment about a common excuse for using it. In this post I give an example of such usage, and discuss whether it is a good idea or not.

Say you are going to write a new class CppInfo, which will print out some information about the C++ language. Here is an example:

class CppInfo
{
public:
    string info()
    {
        return "Language: C++\nCreator : Bjarne Stroustrup\nQuality : ??";
    }
};

Now we need some way to assess the quality of C++, to replace those question marks. Luckily, there exists a class that assesses the quality of a programming language:

class LanguageQuality
{
public:
    virtual ~LanguageQuality() {}

    double quality() const
    {
        return 100.0 / charsForHelloWorld();
    }
protected:
    virtual int charsForHelloWorld() const = 0;
};

As you can see, the only thing we need to do is to create a derived class from LanguageQuality for C++, returning the number of characters required for a C++ Hello, World program.

This is where private inheritance can come in handy. We need access to LanguageQuality::quality(), and we need to override charsForHelloWorld(). If we chose to inherit privately from it in CppQuality, we get both of these, yet none of the members of LanguageQuality will be accessible by the users of that class (as shown in the last post).

Here is a version of CppInfo that inherits privately from LanguageQuality:

class CppInfo_UsingInheritance : private LanguageQuality
{
public:
    string info()
    {
        return "Language: C++\nCreator : Bjarne Stroustrup\nQuality : " + to_string(quality());
    }
private:
    int charsForHelloWorld() const
    {
        return string("#include <iostream>\nint main() { std::cout << "Hello World"; }").size();
    }
};

I have to admit, this looks pretty neat. The alternative is to create a new class whose only responsibilty is overriding charsForHelloWorld():

class CppQuality : public LanguageQuality
{
protected:
    int charsForHelloWorld() const 
    {
        return string("#include <iostream>\nint main() { std::cout << \"Hello World\"; }").size();
    }
};

And then have this as a member in CppInfo:

class CppInfo_UsingComposition 
{
public:
    string info()
    {
        return "Language: C++\nCreator : Bjarne Stroustrup\nQuality : " + to_string(cppQuality.quality());
    }
private:
    CppQuality cppQuality;
};

Now you have two classes instead of one, and 18 lines of code instead of 13. Which solution is best? I am not sure, so let’s have a look at the arguments:

The solution using inheritance is smaller. All the code is in one place, so it is easy to get an overview. We have however coupled our class CppInfo very tightly to LanguageQuality, even though their responsibilities aren’t directly related. Considering the single responsibility principle, CppInfo is about printing general info about C++, whereas LanguageQuality is about computing the quality of a programming language.

I think the principle that provides the tipping point for me in this example, is that every piece of your software should have a single reason to change. Say we want to change how we compute the quality of a language (the number of characters required for Hello, World just doesn’t cut it anymore). This would require a change to LanguageQuality. We would then probably need to update all its derived classes. And to change a class, you really should understand all its responsibilities, invariants and effects. All the other code in CppInfo has nothing to do with the change we are making, so we shouldn’t have to worry about it. Also, if we want to change what information we print out about C++ (say we want to add the publication year of the latest language standard), we want to understand how info is printed, not how the quality is computed.

Again, in a small example like this, it might not matter much, but in a larger example it will. And what starts out small has a nasty tendency to grow, so you’d better care about design from the very start.

Just for fun, here is the result after adding JavaInfo and PythonInfo:

Language: C++
Creator : Bjarne Stroustrup
Quality : 1.587302
Language: Java
Creator : James Gosling
Quality : 0.970874
Language: Python
Creator : Guido van Rossum
Quality : 5.263158

Unsurprisingly, we beat Java, but lose to Python.

Now I am interested to hear your thoughts, which solution would you choose, and why?

As usual, the code for this blog post is available on GitHub.

If you enjoyed this post, you can subscribe to my blog, or follow me on Twitter.

Private Inheritance


In which I introduce private inheritance, but discourage its use.

When inheriting in C++, you normally see

class Derived : public Base {};

It’s almost as if public is synonymous to inherits from. But did you know there is also private inheritance, and why you (probably) don’t see it a lot?

When inheriting publicly from a base class, all base members will be accessible from the derived class, with the same accessibility as in the base class. Given these classes:

class Base
{
public:
    void pub() {}
private:
    void priv() {}
};

class DerivedPublic : public Base
{
};

class DerivedPrivate : private Base
{
};

Public inheritance results in this:

    DerivedPublic derivedPublic;
    derivedPublic.pub();
    //derivedPublic.priv(); //error: ‘void Base::priv()’ is private

Whereas private inheritance results in this:

    DerivedPrivate derivedPrivate;
    //derivedPrivate.pub(); //error: ‘void Base::pub()’ is inaccessible
    //derivedPrivate.priv(); //error: ‘void Base::priv()’ is private

So why would you want to inherit privately? To allow Derived to access the public members of Base, without exposing them to the users of Derived.

Inside the class, the members are accessible:

class DerivedPrivate2: private Base
{
public:
    void foo() { pub(); }
};

But outside, they are not:

    DerivedPrivate2 derivedPrivate2;
    derivedPrivate2.foo();
    //derivedPrivate2.pub(); //error: ‘void Base::pub()’ is inaccessible

But wait a minute, doesn’t this look a whole lot like the good old inheritance (is-a) vs. composition (has-a)? It does indeed! Private inheritance is really a has-a. And in most circumstances composition and private inheritance are interchangeable. However, since inheritance results in stronger coupling, the general recommendation is to choose composition instead. Here is how DerivedPrivate2 would look using composition:

class NotDerived
{
public:
    void foo() { b.pub(); }
private:
    Base b;
};

Now you may be thinking: “But I read somewhere that you need to use private inheritance if you want to override a virtual Base method?” You probably did, and I’ll get back to that in the next post.

As usual, the code for this blog post is available on GitHub.

If you enjoyed this post, you can subscribe to my blog, or follow me on Twitter.

C++ Puzzle #1: Initialization Order


Time for another puzzle! What is the output of this program?


class Parent
{
public:
    Parent(int p=0.0) : p(p)
    {
        cout << "Parent(" << p << ")" << endl;
    }

    int p;
};

class Member
{
public:
    Member(int m=0.0) : m(m)
    {
        cout << "Member(" << m << ")" << endl;
    }

    Member& operator=(const Member& rhs)
    {
            cout << "Member is copied" << endl;
            m = rhs.m;
            return *this;
    }

    int m;
};

class Derived : public Parent
{
public:
    Derived() : foo(10), bar(foo*2)
    {
        Parent(7);
        m = Member(42);
    }

    int bar;
    int foo;
    Member m;
};


int main()
{
    Derived d;
    cout << d.p << " " << d.foo << " " << d.bar << " " << d.m.m << endl;
}

Answer: Undefined behaviour! Which means it could output whatever, crash at any point, or format your hard drive. Can you spot the source of the undefined behaviour? Hint: Look for the use of an uninitialized variable.

Did you see that the order of foo and bar in the initializer list is not in the same order as their declarations? Members are always initialized in the order of declaration, not in the order in the initialization list. This means bar is initialized before foo, using an uninitialized foo to compute its value. If you compile with all warnings enabled (always a good idea), your compiler should warn you about this. For instance, g++ tells me:

main.cpp: In constructor ‘Derived::Derived()’:
main.cpp:29:9: warning: ‘Derived::foo’ will be initialized after [-Wreorder]
main.cpp:28:9: warning:   ‘int Derived::bar’ [-Wreorder]
main.cpp:22:5: warning:   when initialized here [-Wreorder]

Next question then, what is the likely output of this program on a real compiler? On my Ubuntu 12.04 using g++ 4.6, I get:

   
Parent(0)
Member(0)
Parent(7)
Member(42)
Member is copied
0 10 8393920 42  

Lets’ walk through what happens here.

  • 1: Derived inherits from Parent. C++ guarantees that all parent objects are fully constructed before the constructor of the derived class is invoked, so the first thing that happens is that the default constructor of Parent is called. (Did I fool you with Parent(7); in the constructor though? That line will just create a local object that is never used. Had I moved it to the initializer list, it would have been used instead of the default constructor.)
  • 2: Before the body of Derived‘s constructor, all its members will be initialized. Since we don’t specifically initialize Member m in the initializer list, it is first automatically default constructed, and then re-assigned in the body of the constructor (as seen in line 5 of the output).
  • 3: On line 34 of the program, we create a local Parent object which is never used. Maybe we meant to set p to 7, but put the Parent() call in the wrong place?
  • 4-5: Now another Member is constructed, and copy assigned to m. All this re-construction and copying could have been avoided if we had moved the call Member(42) to the initializer list.
  • 6: Finally, we have a look at the resulting values of Derived‘s members:
    • p is 0, not 7 as we maybe meant it to be, as explained in 1.
    • foo is 10, hopefully as expected.
    • bar is 8393920, due to the use of the uninitialized foo, as explained earlier. This is entirely by chance, and should not be relied on! Your program might just as well crash, or do something worse.

Finally let’s clean up the program and see what happens. We reorder the declarations of foo and bar, and move all initializations to the initializer list:

ERROR: Couldn't open file: [Errno 2] No such file or directory: '/home/anders/Documents/code/blog/initialization/main2.cpp'

Now the program is well defined, and free of repeated constructors and copying. Here is the output:

Parent(7)
Member(42)
7 10 20 42

Much better! Full source here.

As usual, the code for this blog post is available on GitHub.

If you enjoyed this post, you can subscribe to my blog, or follow me on Twitter.

Modify Visibility in a Subclass to Allow Testing


In which I argue it might sometimes be useful to test private/protected methods, and demonstrate how to do it in a framework-independent way.

How do you test private/protected functions? The most common, and often correct answer is “Don’t!”. But sometimes it can be useful, especially when test-driving helper-methods. Example:

class Ohlson {
protected:
	int helper(); //We want to test this
public:
	int compute() {
		//(...)
		int n = helper();
		//(...)
	}
};


TEST(TestOhlson, helper_returns42) {
	Ohlson o;
	ASSERT_EQ(42, o.helper());
}

This will fail: error: ‘int Ohlson::helper()’ is protected. Many unittesting frameworks provide a way to work around this, like googletest’s FRIEND_TEST, but there is a way to get around this in normal C++, by subclassing to modify visibility.

In C++, a subclass can change the visibility of a derived function. This is not possible in Java or C# as far as I know, and to be honest it does sound somewhat dangerous. But C++ wasn’t made to keep you in a padded box with a helmet on, was it?

Here’s how you do it:

class OhlsonForTest : public Ohlson {
public:
	using Ohlson::helper;
};

And voilà, helper() is now public and can be tested:

TEST(TestOhlson, helper_returns42) {
	OhlsonForTest o;
	ASSERT_EQ(42, o.helper());
}

This trick can be useful in some situations, but when you are done, see if you might refactor your way out of the problem instead.

And I would be extremely sceptical to do anything like this in production code!

By the way, this trick is often useful in combination with last weeks technique on Making a Derived Class to Allow Testing an Abstract Class.

If you enjoyed this post, you can subscribe to this blog, or follow me on Twitter

Make a Derived Class to Allow Testing an Abstract Class


In which I show how best to test an abstract class, which normally wouldn’t be instantiatable.

Abstract classes are not instantiatable, so how do you test one?

class Abstract
{
public:
	int common();
	virtual int special() = 0;
};

class Concrete : public Abstract
{
public:
	int special();
};

Say you want to test Abstract::common(), like so:

TEST(TestAbstract, helper_returns42)
{
	Abstract abs; //cannot declare variable ‘abs’ to be of abstract type ‘Abstract’
	ASSERT_EQ(42, abs.common());
}

It’s not possible, because common() special() is pure virtual (it has no implementation in Abstract). So how do you test a class that cannot be instantiated? It can be tempting to test through a concrete subclass:

TEST(TestAbstract, common_returns42)
{
	Concrete abs; 
	ASSERT_EQ(42, abs.common());
}

But this is generally a bad idea, since Concrete might change the behaviour of common(). Even though it is not virtual, it operates under a different state. And even though you know it will work now, you don’t know how Concrete will change in the future, and then you don’t want tests that don’t have anything to do with Concrete to start failing. Also, creating a Concrete object when you are testing on Abstract is confusing to the reader.

Instead, I recommend you derive a class to be used just for testing:


class AbstractForTest : public Abstract {
	int special();
};

TEST(TestAbstract, common_returns42)
{
	AbstractForTest abs; 
	ASSERT_EQ(42, abs.common());
}

In this way you get rid of the unwanted dependency, and the test is easier to understand. As a final note, make sure you name your test-class something that clearly distinguishes it as a testing artifact, like I did above. Also make sure to keep it in your test code (e.g. TestAbstract.cpp), not in your production code (e.g. Abstract.h/.cpp).

Static and Dynamic Polymorphism – Siblings That Don’t Play Well Together


Static and dynamic polymorphism might sound related, but do not play well together, a fact I had to deal with l this week.

This is the situation: We have a set of worker classes, that work on a set of data classes. The worker classes can do different types of work, but the type of work is always known at compile time, so they were created using static polymorphism, aka templates.

template &lt;typenmame WorkImpl&gt;
class Worker {
...
};

WorkImpl decides the details of the work, but the algorithm is defined in Worker, and is always the same.

Those are the workers. We then have a pretty big tree of data classes. What kind of objects we need to deal with is not known until runtime, so we are using inheritance and virtual methods for these.

  A
 / \
B   C
|
D 

etc.

A has a virtual method fill() that gets passed a WorkImpl object, and fills it with data. fill() needs to be implemented in all the subclasses of A, to fill it with their special data. Bascially, what we want is to say

class A { 
  template  &lt;typename WorkImpl&gt;
  virtual fill(WorkImpl&amp; w) {...} //overridden in B, C, D etc.
};

Can you spot the problem?

Hint: When will the templated fill() methods be instantiated?

There is no way for the compiler to know that classes A-D will ever have their fill()-methods called, thus the fill()-methods will never be instantiated!

One solution to this problem is to make one version of fill() per type of WorkImpl, in each data class A-D. The body of a fill() method is independent of the type of WorkImpl, so this solution would lead to a lot of unnecessary duplication.

The solution we ended up with was creating wrapper methods that take a specific WorkImpl, and just calls fill() with that argument:

class A {
  virtual fillWrapper(OneWorkImpl&amp; w) { fill(w) };
  virtual fillWrapper(AnotherWorkImpl&amp; w) { fill(w) };
  template &lt;typename WorkImpl&gt;
  virtual fill(WorkImpl&amp; w) {...}
}

In this way we minimize code duplication, and still make sure the fill() methods are always instantiated.

Question: Can we get away with defining the wrappers in A only? And if not, when will it fail?

Answer: We can not. If we only define the wrappers in A, fill() will never be instantiated in B-D. Everything compiles and links, but at runtime, it is the A version that will be called for all the classes, leading to a wrong result.

Finally, I should mention that another option would have been to refactor the worker classes to use dynamic polymorphism, which would basically solve everything, and allow us to get rid of the wrapper functions. But that would have been a bigger job, which we didn’t have time for.

Why Initializer Lists don’t Work with Base Members


As you might know, you cannot initialize a base member in a derived constructors initializer list:

struct Base {
    int base_mem;
};

struct Derived : public Base {
    Derived(int i) : base_mem(i) {} //Error!
};

int main() {
    Derived(42);
}

This is not allowed. But why? There are two key points to get here:

1: What is the point of an initializer list in the first place?
If you don’t use an initializer list, but initialize all your objects in the constructor body, they will first be created once, even before the constructor body, and then you will re-assign them. This might be a performance-problem, but could also be a bigger problem if the constructors of those types have side-effects like hitting a database, acquiring some resource etc.

2: When are the different parts of an object constructed?
When constructing a Derived object, the first thing that happens is that Bases default constructor is invoked. In this example, I didn’t declare one, so the compiler will generate one for me. When this constructor is done, including both its initializer-list and its body, it is Derived()s turn.

This means that when C++ gets around to Derived()s initializer, the Base part of the object has already been created, all Base members are initialized, and we have lost the advantage of using initializer lists.

So how do we solve this? Make sure the Derived constructor explicitly specifies which Base constructor to use, and have this one initialize the variable. Example:

struct Base {
    Base(int i) : base_mem(i) {}
    int base_mem;
};

struct Derived : public Base {
    Derived(int i) : Base(i) {}
};