oop: Composition or Inheritance in concrete case

We've just had a discussion with college about is the following style acceptable for oop or not.

We've a class, which has one public function, and requires a reader in the constructor:

public class Converter
{
    private readonly IReader _reader;

    public Converter(IReader reader)
    {
        _reader = reader;
    }

    public byte[] Convert(...params...)
    {
         return something;
    }
}

We have Reader1 and Reader2 which both implement an IReader.

I want to setup two managers: Converter1 and Converter2, providing the same public Convert() function, but Converter1 will be using Reader1, and Converter2 will use Reader2.

For me the simplest solution is to inherit from Converter and initialize it with proper reader:

public class Converter1 : Converter
{
    public Converter1():base(new Reader1())
    {}
}
public class Converter2 : Converter
{
    public Converter2():base(new Reader2())
    {}
}

My college says, that Converter1 and Converter2 are Managers, and inheritance should not be used for managers, instead we should apply a composition here. But from my persp开发者_StackOverflow中文版ective composition will only result in additional code in specific converters classes.

So, could you please advice, whether it is ok to use inheritance when implementing managers or not? Thanks

Why are you inheriting at all??

Based on the sample you have provided, you are not doing anything additional to the base class other than ensuring that Converter1/Converter2 enforces a specific type of reader.

It seems to me that your collegue is right. What you should be doing is implementing a Factory Method, that will create and populate the correctly configured Converter for you.

i.e.

public static class ConverterFactory {
    public static CreateConverter1() { 
       return new Converter(new Reader1());
    }
    public static CreateConverter2() {
       return new Converter(new Reader2());
    }
}

...
Converter x = ConverterFactory.CreateConverter1();

To my mind, this is an abuse of inheritance. You aren't specializing the behaviour of the converter - you're only specializing the construction.

In particular, you could easily have a static class with static methods to perform this construction:

public static class Converters
{
    public static Converter CreateConverter1()
    {
        return new Converter(new Reader1());
    }

    public static Converter CreateConverter2()
    {
        return new Converter(new Reader2());
    }
}

These could even be static methods within the normal Converter class, of course.

The fact that this loses no functionality suggests to me that inheritance was a mistake.

Then again, I'm regularly suspicious of inheritance. Designing inheritance properly means working out the extension points, documenting how they should behave - which is a balancing act between giving callers enough information to predict consistent behaviour, and giving implementors enough wiggle room to vary the behaviour in useful ways. Here you're not doing any of this - you're just changing what reader is passed to the constructor.

Either use inheritance (like you proposed), or just use one Converter class which will work with the polymorphic IReader.