OOP Gone Wild

In a piece of code I inherited from some smart, well meaning, but a little disorganized East European consultants, I found the following class hierarchy:

RestClient (abstract)
   JsonClient (abstract)
      XyzClient*
          SessionClient
      ApiClient


* XYZ is our proprietary service. The name was changed to protect the innocent.

It’s been a while since I’ve seen a class hierarchy 4 levels deep, especially with concrete classes inheriting from other concrete classes. Somehow these days people prefer composition. None of the abstract classes contain any abstract methods, just protected virtuals with implementations. So why are those classes abstract then?

I believe someone long time ago established that inheriting concrete classes from other concrete classes is a bad idea, but I already forget who, and I hope his advice was not based on the C++ “slicing” phenomenon.

The separation of responsibilities between the classes is somewhat unclear. ApiClient is described as “Base client class for REST API clients that manages XYZ session”. I wonder then what’s the difference between it and the SessionClient. Anyhow, it probably does not matter, since ApiClient is not used anywhere 🙂 In fact, deeper investigation shows that all of these classes are dead code. We do have a place that makes a REST call and parses JSON, but it does not use any of those classes, and neither does any other piece of callable code.

Don’t do this at home, folks.

Leave a Reply

Your email address will not be published. Required fields are marked *