CodeSOD: Wages of Inheritance

Tim H writes:

Some say that OOP was the greatest mistake of all. I say they weren’t trying hard enough.

This code is C++, though Tim submits it as "C with classes." That usually means "we write it as much like C as possible, but use classes to organize our modules." In this case, I think it means "we use classes to punish all who read our code".

Let’s look at an example. They’ve been anonymized, but the shape of the code is there.

class Base {
public:
enum class Type {
derived_1,
derived_2
};

Base(Type t) : t_{t} {}

Type getType() const { return t_; }

private:
Type t_;
};

class Derived_1 : public Base {
public:
Derived_1() : Base(Base::Type::derived_1) {}
};

This is what one might call "inheritance". You shouldn’t, but you might. Here, the ba…

Tim H writes:

Some say that OOP was the greatest mistake of all. I say they weren’t trying hard enough.

Let’s look at an example. They’ve been anonymized, but the shape of the code is there.

class Base {
public:
enum class Type {
derived_1,
derived_2
};

Base(Type t) : t_{t} {}

Type getType() const { return t_; }

private:
Type t_;
};

class Derived_1 : public Base {
public:
Derived_1() : Base(Base::Type::derived_1) {}
};

This is what one might call "inheritance". You shouldn’t, but you might. Here, the base class has an enumerated type which declares the possible child classes, and a field to hold that type. The child classes, then, must set that type when they’re constructed.

This is inheritance and polymorphism implemented from first principles, badly. And you can see how badly when it comes time to use the classes:

void Foo(Base *b) {
if(b->getType() == Base::Type::derived_1) {
// do it
}
}

That’s right, they need to check the type field and branch, instead of leveraging polymorphism at all.

But this isn’t the only way they’ve reinvented inheritance. I mean, why limit yourself to just one wrong way of doing things, when you can use two wrong ways of doing things?

class Derived_1;
class Derived_2;

class Base {
public:
Derived_1* getDerived_1() {
return dynamic_cast<Derived_1*>(this);
}
Derived_1& getDerived_1_ref() {
return dynamic_cast<Derived_1&>(this);
}
};

class Derived_1 : public Base {}

Here, the base class implements methods to get instances of child classes, or more accurately, pointers (or references) to instances of the child class… by applying the cast to itself. The base class contains logic which casts it to a child class.

Once again, we’ve reinvented a kind of inheritance which requires the base class to know about all its derived classes. I believe the intended use here is that you may have a variable of Base* that is pointing to an instance of Derived_1, and you want to cast it to access the derived class. The problem is that it may be a pointer to Derived_2, so what happens when you ask for getDerived_1()? dynamic_cast will check the runtime type information- if you compiled with RTTI enabled. Otherwise you’re flirting with nasal goblins.

Tim writes:

These "idioms" are inconsistently used everywhere in the codebase. The member function names really do have the "_ref" for the return-a-ref version.

Now, I know that they are running with RTTI enabled, and thus when they do a bad cast, dynamic_cast will throw a bad_cast exception? How do I know?

Double fun is when users report an unreproducible std::bad_cast.

As Tim points out, they do get bad_cast exceptions. And because this is a pile of bad choices and risky code, they can’t trace why it happened or how.

Sometimes, things just blow up.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

Similar Posts