Search code examples
c#ooppolymorphismsolid-principles

Is empty implementation and unused parameter in abstract methods a good approach in certain cases?


I have a situation where I need to parse objects of different types. Most of the types can be parsed in a generic way with the exception of certain types, which require extra filling. For the sake of simplicity, let's assume there is one type which requires performing some special operations, and lets call its parser SpecialParser.

In the current implementation, I have a base class called BaseParser, which implements an interface ICustomParser. A generic parser capable of parsing all the regular types called GenericParser and the SpecialParser. Both the GenericParser and the SpecialParser inherit from the BaseParser class which implements basic common functionalities. All the parsers are referenced using the interface and called through a public method implemented in the BaseParser. The type of the parser to use is determined using a property implemented in each parser which indicates the types it can parse.

The function which builds the parsed object (BuildParsedObject()) has some common parts in all the parsers, but requires some additional logic in the special type parsers.

Currently, the BaseParser has an implementation which looks like this:

public abstract class BaseParser : ICustomParser
{
    // Common methods and functionalities

    protected abstract Type GetParsedType(string typeIdentifier);
    protected abstract void FillConcreteProperties(...);

    public Object BuildParsedObject(JObject objectToBuild)
    {
        // Some common logic.

        var parsedType = GetParsedType(typeIdentifier);

        // Call the specific filling logic if needed. 
        // The order here is important. The concrete filling must be done before the general one.
        FillConcreteProperties(...); 

        // (More common logic) 
        // Fill the general properties for all the types. This method is implemented here in the base class.
        FillGeneralProperties(...);  

        // Cast the object to the parsedType and return it.
        return objectToBuild.ToObject(parsedType);
    }
}

The GenericParser looks like this:

public class GenericParser
{
    protected override Type GetParsedType(string typeIdentifier)
    {
        // Perform certain checks on the given <typeIdentifier> and logic in 
        // order to determine the parsed object type and return it.
    }

    protected override void FillConcreteProperties(...)
    { // The implementation is empty here since no special filling is required for the regular types.
    }
}

And the SpecialParser looks like this:

public class SpecialParser
{
    protected override Type GetParsedType(string typeIdentifier)
    {
        // The <typeIdentifier> parameter is never used, since the type is already known.
        // So we always just return a hardcoded type for the parser (for e.g. A).
        return typeof(A);
    }

    protected override void FillConcreteProperties(...)
    {
        // Contains an implementation for the specific type parsing and fills the object being built accordingly.
    }
}

The advantage of the current implementation is that all the parsers only need to implement the GetParsedType() and the FillConcreteProperties() methods. All the parsing logic is shared and done at the base class level, and we don't need to replicate the implementation of the BuildParsedObject() method.

The disadvantage is that the GenericParser doesn't need to implement the FillConcreteProperties() method, so we end up with an empty implementation there. And the special parsers don't need to perform a logic in order to determine the output type, so they end up implementing a dummy GetParsedType() method and not using the passed argument there.

Is it fine to have an empty implementation of an abstract method? And an unused parameter in an implemented abstract method? Or is it a bad practice and should be avoided?

If it should be avoided, is it a good idea to move the implementation of the BuildParsedObject() to the child parser classes and have each one implement it as needed and remove the need to have empty implementation of the FillConcreteProperties() and the unused parameter? despite that it would create some kind of duplication there?


Solution

  • Is it fine to have an empty implementation of an abstract method?

    Not really. It is usually an indication of a violation of the Liskov substitution principle.

    And an unused parameter in an implemented abstract method? Or is it a bad practice and should be avoided?

    This is also a code smell, indicating that there is low cohesion in your class.

    What you could do, is check the decorator pattern of the gang of four. That way you can create extended decorators that will enrich your object according to your business logic.

    You can remove the unneeded function this way, run the base function and then enrich your object with whatever new information you may need.