This was another day of no runtime bugs. Instead of a picture, I’ll describe a specific(ish) case of using the compiler to catch all uses of a particular pattern in the code.
class Foo {};
// Bar is one of many classes that inherit from Foo
class Bar : public Foo {};
// Baz is one of many classes that inherit from Bar
class Baz : public Bar {};
DECLARE_EXTRA_STUFF_FOR_CLASS(Baz);
So we have something like the above. The DECLARE_EXTRA_STUFF_FOR_CLASS() macro is used for some but not all classes that inherit directly or indirectly from Foo.
There have recently been changes within a system that means that DECLARE_EXTRA_STUFF_FOR_CLASS() is now never needed for any class that inherits from Bar — but it is not invalid for it to remain used in the codebase. Anything that makes use of the EXTRA_STUFF that the macro provides will still work, it’s just extra unneeded code which I’d like to clean up.
I could audit this by hand — find every class inheriting from Bar, check to see if there’s a macro definition for that class somewhere and perform the subsequent clean up, but I’m lazier than that.
I could write a script that finds classes that inherit from Bar, and then search for that class name with the macro, but that also seems like to much work. (A simple regex sweep through the codebase won’t trivially identify class Asd : public Baz {} as inheriting from Bar). I’m lazier than this.
What I can do is break the code expanded by the macro if it is passed a subclass of Bar. To do that, I can do something like the following:
class Foo { public: static void Foo(){} };
class Bar : public Foo { private: static void Foo(); };
// Add a line to the macro...
DECLARE_EXTRA_STUFF_FOR_CLASS(class_name) \
extra_stuff; \
class_name::Foo(); \
more_extra_stuff
Done. Anywhere where the macro is invoked for a class inheriting from Bar will trigger a compile error. Walk through the errors, delete calls to that macro, rewrite (i.e. delete) code as necessary.
End result: less code. I count that as a win.
(In my specific case, I made an already existing public method declared in Foo private in Bar because there is no need to call it from or for classes in that part of the hierarchy. This has the added advantage of not polluting the list of Foo‘s members with no functionality of the class. Additionally, I didn’t call the function within the macro but instead took it’s address.)
If
Foo::Foo()is never needed inBarand it’s subclasses, isn’t that a hint that there are two parts of functionality inFoo'sfunctionality that should be separated? And if that’s the case, why not just extract the common parts to e.g.FooBarBaseand derive bothFooandBarfrom that class. Deriving and “privatizing” part of the inherited interface imo somehow violates the Liskov Substitution Principle, by making the inheritance an “is-not-exactly-a” relationship.Admittedly, that need not apply to a static member function, so I am not sure if my gut feeling is correct here.
You are correct in saying that there are parts of Foo’s functionality that could be separated, though as to whether they *should* be separated is another question. I had proposed such a change (in practice, there’s about four distinct categories of things that Foo defines that I would like to see split into distinct classes), but the primary users of Foo’s many, many subclasses have expressed opposition to it.
Sometimes design principles takes second place to user preference :)