Is it bad practice to mix SubTyping and Overriding?

Question

Please correct me of any of statements in this thread are wrong.

I read that reusing code is the primary benefit of inheritance, but it dawned on me that polymorphism through subtyping is the primary advantage of inheritance and makes it possible to reuse code in complex systems.

In many articles online, I see subtyping constantly combined with polymorphism. I understand that subtyping is a means to achieve overriding in inheritance patterns. Although, both are different forms of polymorphism.

Link in the below code as an example:

class Logger
{
    public void count(string filepath) {
        Writeline("I'm counting lines of %s pretty hard!\n", $filepath);
    }
}

class JSONLogger extends Logger {
    public override void count(string filepath) {
        Writeline("I'm counting JSON objects from file %s pretty hard!\n", $filepath);
    }
}

class Shipment
{
    private Logger logger;

    public Shipment (Logger logger) {
        this.logger = logger;
    }

    public void count(string filepath) {
        this.logger.count(filepath);
    }
}

shipment = new Shipment(new JSONLogger());
shipment.count("/my/shipment");

Here, we use a subtype of Logger, that is JSON Logger, to override the count() method.

But was subtyping meant to be used with overriding? Isn't mixing overriding and subtyping bad?

For example, the user who wants to create an instance of Shipment will need to understand what Logger to us? A regular logger or a JSONLogger?

The user will need to look at how both these loggers are implemented to decide on which behavior he needs to use. But doesn't this violate the principle of abstraction?

We create objects and abstract behavior so we don't need to worry about how they are implemented. If the above code was a more complex inheritance tree with more classes and more overriding, then we need to know all this extra information about how sub-class override super class behavior and if the superclasses themselves overrode any base class behavior, etc.

In this case, isn't it actually hard to practically implement subtyping or substitute objects dynamically during run time?

So, is mixing overriding and subtyping bad? or is overriding in general just bad design?

EDIT: Reason I had these questions was because I read an article about the LSP principle by Barbara Liskov. She states that

Objects of subtypes should behave like those of supertypes if used via supertype methods

According to this statement, if we want to use polymorphism together with inheritance, we need to have proper subtyping by following this rule: when you substitute one superclass by its subclass, the behavior of the whole system should be unchanged.

Doesn't Overriding change the behavior since you have a new implementation of the superclass method in the subclass?

Answer 1

I think you are asking the wrong questions and using the wrong terminologies. Overriding actually refers to the act of re-implementing a superclass method in a subclass and it's actually quite common. There's no more inherent issue than with inheritance itself. However, if you are overriding all the methods without super calls then inheritance becomes pointless as there's no inherited behavior. In the above example, Logger would most likely be an interface rather than a class and you'd have 2 concrete implementations of that interface.

Here, we use a SubType of Logger, that is JSON Logger, to override the count method.

That's not an override. Delegating the implementation to an injected collaborator is referred to as the Strategy Pattern and it's one of the most widely used design pattern.

For example, the user who wants to create an instance of Shipment will need to understand what Logger to us? A regular logger or a JSONLogger?

Often times, the client actually wants to be in control of the strategy. For instance, when you create a TreeSet in java (sorted set), you may pass a Comparator to be used for custom sorting. However, there are also cases where you want to relieve the client from making such decisions explicitly and you might abstract away that part of the creation process using one of the creational patterns.

As a side note, I understand your example is most likely not taken from the real world, but a problem with the design seems to be that Shipment has too many responsibilities and works with collaborators at the wrong level of abstraction. A Shipment most likely shouldn't have any knowledge of file paths, etc.

Answer 2

Inheritance is a language tool that is used to achieve multiple things, depending on the need:

• It can be used for polymorphism - this is where you define an API in the superclass and implement it in different ways in different subclasses (e.g. production implementation vs in-memory implementation for unit tests). The superclass would usually be a pure interface.

• It can be used to reuse code, e.g. via Template Method design pattern. While this is common, it isn't a great way to share code, and delegation should be preferred to inheritance in this case - Inheritance creates a dependency of the subclass on the superclass, and in most cases such dependency isn't needed.

• It can be used to reduce and reuse boilerplate methods. For example, if your classes Employee, Student, Baby all have a getName(), getAge(), setPicture() etc., then you wouldn't want their common superclass Person to be an interface - you'll want to implement these methods in the Person itself. (this is a case where using delegation will create a ton of boilerplate.)

So you can consider certain patterns - such as inappropriate use of inheritance for code reuse - as bad design, regardless of polymorphism.
LSP, on the other hand, is concerned with polymorphism, and doesn't really apply to inheritance used for code/boilerplate reuse.

In the shipment and logger example, the use case is polymorphism and is subject to LSP. You have multiple loggers implementing the same API such that Shipment doesn't care about their implementation.

You aren't really reusing code between implementations, and it would have been more appropriate to have a ILogger interface, and have Logger and JSONLogger directly subclass ILogger.

Then there are what seem to be the 2 core issues in the question:

Defaults: What if the caller doesn't want to care about the specific logger implementation?
In case you want to provide a default implementation for every class that uses the would-be ILogger, you could have a static method in ILogger that returns a default logger.
If, instead of a logger class, you'd have something larger, with a lot of logic, that only needs to customize a small part, and you choose to go with template methods for customization (Logger.count() in this example) rather than delegation, then providing a default implementation for the template method would be appropriate. But it's important to clarify to implementers of Logger subtypes that it's a default implementation.

Whether or not it is a violation of abstraction to not have a default, depends on where and how the class is created - usually, you want to abstract away details from the code that uses Shipment, but not necessarily from the code that creates Shipment (with a particular logger) - if your design can't do that, then the code that uses Shipment will have to care about logger, which would indeed violate abstraction.
This is why it is typical to use the Abstract Factory pattern. They allow selection of the appropriate class under the appropriate conditions. They can e.g. re-read a config file and create a different logger based on new configuration.

Sharing code among subtypes: If you are trying to achieve polymorphism, but a lot of the implementation is common among subtypes:
If that common code is a lot of actual logic, then just use delegation - create a library/helper that provides the common logic, and have each implementation call whatever APIs it needs.
If that common code is a lot of boilerplate, then it's preferable to use inheritance. This is a case where mixing polymorphism with reuse is not bad. Common practice is to have an ILogger interface, an AbstractLogger that inherits from ILogger and implements all the boilerplate, and have regular implementations such as Logger and JSONLogger inherit from AbstractLogger and complete the non-boilerplate parts.
This keeps the interface fully abstract and allows implementations that have other boilerplate (e.g. have count() throw an exception for testing).

Something like AbstractLogger by definition implements the template method pattern, and as such may provide defaults.
Instead of a default, it can provide a partial/minimal implementation, and request that overrides of count() call super.count() (that should be in the API documentation of the protected method). This is precisely meant to maintain the LSP invariant. But that's not a very good way to do it - it's better to make count() a concrete method that calls a new template method, and ask subclasses to implement that new template method.