13

I have a class with some default/shared functionality. I use abstract class for it:

public interface ITypeNameMapper
{
    string Map(TypeDefinition typeDefinition);
}

public abstract class TypeNameMapper : ITypeNameMapper
{
    public virtual string Map(TypeDefinition typeDefinition)
    {
        if (typeDefinition is ClassDefinition classDefinition)
        {
            return Map(classDefinition);
        }
        ...

        throw new ArgumentOutOfRangeException(nameof(typeDefinition));
    }

    protected abstract string Map(ClassDefinition classDefinition);
}

As you can see, I also have the interface ITypeNameMapper. Does it make sense to define this interface if I already have an abstract class TypeNameMapper or abstract class is just enough?

TypeDefinition in this minimal example is abstract too.

Deduplicator
  • 8,591
  • 5
  • 31
  • 50
Konrad
  • 1,529
  • 2
  • 17
  • 32
  • 4
    Just FYI - in OOD, checking types in code is an indication that your class hierarchy is not correct - it is telling you to create derived classes from the type you are checking. So, in this case, you want an interface ITypeMapper that specifies a Map() method and then implement that interface in both the TypeDefinition and ClassDefinition classes. That way, your ClassAssembler simply iterates through the list of ITypeMappers, calling Map() on each, and gets the desired string from both types because they each have their own implementation. – Rodney P. Barbati Jul 26 '18 at 18:35
  • 1
    Using a base class in place of an interface, unless you absolutely have to, is called "burning the base". If it can be done with an interface, do it with an interface. The abstract class then becomes a helpful extra. https://www.artima.com/intv/dotnet.html Specifically, remoting requires you to derive from MarshalByRefObject so if you want to be remoted you *cannot* derive from anything else. – Ben Jul 26 '18 at 19:42
  • @RodneyP.Barbati won't work if I want to have many mappers for the same type that I can switch depending on the situation – Konrad Jul 26 '18 at 22:04
  • 1
    @Ben burning the base that's interesting. Thanks for the gem! – Konrad Jul 26 '18 at 22:15
  • @Konrad That is incorrect - there is nothing preventing you from implementing the interface by calling another implementation of the interface, hence, each type may have a pluggable implementation that you expose via the implementation in the type. – Rodney P. Barbati Aug 05 '18 at 20:46
  • @RodneyP.Barbati do you mean a method like `Map(ITypeNameMapper mapper)` ? Or exposing the mapper property like `ITypeNameMapper Mapper { get; set; }` directly in the type. It's also fine. – Konrad Aug 05 '18 at 20:54
  • @RodneyP.Barbati could you give a code example of what do you mean? – Konrad Aug 05 '18 at 21:16
  • In a class implementing ITypeNameMapper you have a collection of ITypeNameMappers and somehow select the desired one. In the implementation of the Map() method, you return the result of calling Map() on one of the ITypeNameMappers in your collection. – Rodney P. Barbati Aug 05 '18 at 21:24

5 Answers5

32

Yes, because C# doesn't allow multiple inheritance except with interfaces.

So if I have a class which is both a TypeNameMapper and SomethingelseMapper I can do:

class MultiFunctionalClass : ITypeNameMapper, ISomethingelseMapper 
{
    private TypeNameMapper map1
    private SomethingelseMapper map2

    public string Map(TypeDefinition typeDefinition) { return map1.Map(typeDefintion);}

    public string Map(OtherDef otherDef) { return map2.Map(orderDef); }
}
Ewan
  • 70,664
  • 5
  • 76
  • 161
  • I didn't think of the multiple inheritance example... hopefully Single Responsibility will limit the need for this but agree it will add more flexibility – Liath Jul 26 '18 at 13:25
  • What if I don't need multiple inheritance in this particular case? – Konrad Jul 26 '18 at 13:29
  • I also have some other private things `TypeNameMapper` can't live without. So if someone implements `ITypeNameMapper` wrong my other code that expects it somewhere won't work as it should. – Konrad Jul 26 '18 at 13:31
  • Then you will forever regret typing those extra three lines of code. – Ewan Jul 26 '18 at 13:32
  • @Ewan I don't think I will regret. What if I don't want any other implementation of `ITypeNameMapper` ? – Konrad Jul 26 '18 at 13:33
  • 4
    @Liath: Single responsibility is actually a contributing factor as to why inheritance is needed. Consider three possible traits: `ICanFly`, `ICanRun`, `ICanSwim`. You need to create 8 creature classes, one for every combination of traits (YYY, YYN, YNY, ... , NNY, NNN). SRP dicates that you implement each behavior (fly, run, swim) once and then reusably implement them on the 8 creatures. Composition would be even better here, but ignoring composition for a second, you should already see the need for inheriting/implementing multiple separate **reusable** behaviors due to SRP. – Flater Jul 26 '18 at 13:46
  • @Flater I don't think in my case I violate SRP. This class is simply responsible to map implementations of `TypeDefinition` to names (something like `ToString` but depending on the case). I don't think creating interfaces like `IClassDefinitionNameMapper` would make any sense here. – Konrad Jul 26 '18 at 13:52
  • 4
    @Konrad that's what i mean. If you write the interface, and never need it, then the cost is 3 LoC. If you don't write the interface and find you do need it, the cost _might_ be refactoring your entire application. – Ewan Jul 26 '18 at 13:55
  • @Ewan oh yeah that makes more sense now – Konrad Jul 26 '18 at 13:56
  • @Konrad: I'm not sure what you're responding to. I am not claiming that anyone is violating SRP here. I was responding to Liath's mention that SRP _alleviates_ the need for multiple inheritance. – Flater Jul 26 '18 at 13:58
  • @Flater oh nvm then – Konrad Jul 26 '18 at 14:02
  • 3
    (1) Implementing interfaces is inheritance? (2) The question and answer both should be qualified. "It depends." (3) Multiple inheritance needs a warning sticker on it. (4) I could show you K's of craptacular formal `interface` malpractice.. redundant implementations that should be in the base - that are evolving independently!, conditional logic changes driving this junk for want of template methods, broken encapsulation, non-existant abstraction. My favorite: 1-method classes each implementing its own 1-method `interface`, each w/ only a single instance. ... etc, etc, and etc. – radarbob Jul 26 '18 at 20:36
  • @radarbob 1: technically yes 2: It depends answers are dull 3: thats why c# doesn't allow it 4: answers with stories of crappy code from the field are the best answers. plz write one!! – Ewan Jul 26 '18 at 20:41
  • 3
    There is an invisible coefficient of friction in code. You feel it when forced to read superfluous interfaces. Then it shows itself heating up like the space shuttle slamming into the atmosphere as an interface implementation moons the abstract class. It's the Twilight Zone and this is the shuttle Challenger. Accepting my fate I watch the raging plasma with fearless, detached wonder. Remorseless friction rips apart the facade of perfection, depositing the shattered hulk into production with a thunderous boink. – radarbob Jul 26 '18 at 22:01
  • 4
    @radarbob Poetic. ^_^ I'd agree; while the cost of interfaces is low, it's not non-existent. Not so much in writing them, but in a debugging situation it's 1-2 more steps to get to the actual behavior, which can add up quickly when you get a half-dozen levels of abstraction deep. In a library, it's usually worth it. But in an application, refactoring is better than premature generalization. – Errorsatz Jul 27 '18 at 00:18
  • @radarbob (1) They are very close to eachother and for the purpose of this discussion they are the same. Consider the similarities between inheriting a purely abstract class and implementing an interface. (4) I can show you K's of people hurting themselves when they fall to the ground but that doesn't mean that standing upright should be illegal. I agree with the 'don't overengineer' tone you carry but that's a separate discussion. The question is if (_in general_) interfaces _add_ something when abstract classes already exist. The question is not asking whenan interface is warranted. – Flater Jul 27 '18 at 08:06
  • @Flater, "[do] interfaces add ... [to] abstract classes?": My spidey sense is buzzing big time. However If this answer is not construed as license to do this in general then .... ok - but there better be a design justifying and supporting this. Multiple inheritance aside, this "inheritance by interface" isn't what many intend, I suspect. They simply heard "code to interfaces not implementation" and do not grock the formal `interface`s raison d'être, nor the inheritance/interface symbiosis, nor how to adequately integrate it via OO design. – radarbob Jul 27 '18 at 16:51
1

Interfaces and abstract classes serve different purposes:

  • Interfaces define API's and belong to the clients not the implementations.
  • If classes share implementations then you may benefit from an abstract class.

In your example, interface ITypeNameMapper defines the needs of clients and abstract class TypeNameMapper is not adding any value.

Geoffrey Hale
  • 129
  • 1
  • 8
  • 2
    Abstract classes belong to the clients too. I don't know what do you mean by that. Everything that is `public` belong to the client. `TypeNameMapper` is adding a lot of value because it saves the implementer from writing some common logic. – Konrad Jul 27 '18 at 07:10
  • 2
    Whether an interface is presented as an `interface` or not is only relevant in that C# forbids full multiple inheritance. – Deduplicator Jul 27 '18 at 07:19
0

The whole concept of interfaces was created in order to support a family of classes having a shared API.

This explicitly is saying that any use of an interface implies that there is (or is expected to be) more than one implementation of its specification.

DI frameworks have muddied the waters here in that many of them require an interface even though there will only ever be one implementation - to me this is unreasonable overhead for what in most cases is just a more complex and slower means of calling new, but it is what it is.

The answer lies in the question itself. If you have an abstract class, you are preparing for the creation of more than one derived class with a common API. Thus, the use of an interface is clearly indicated.

Rodney P. Barbati
  • 197
  • 4
  • 2
    "If you have an abstract class, ... the use of an interface is clearly indicated." - My conclusion is the opposite: If you have an abstract class, then use it as if it was an interface (if the DI doesn't play with it, consider a different implementation). Only when it really doesn't work, create the interface - you can do it anytime later. – maaartinus Jul 26 '18 at 19:18
  • 1
    Ditto, @maaartinus. And not even " ... as if it was an interface." The public members of a class *is* an interface. Also ditto for "you can do it anytime later"; this goes to the heart of design fundamentals 101. – radarbob Jul 26 '18 at 20:50
  • @maaartinus: If one creates an interface from the start, but uses references of the abstract-class type all over the place, the existence of the interface won't help anything. If, however, client code uses the interface type instead of the abstract-class type whenever possible, that will greatly facilitate things if it becomes necessary to produce an implementation which is internally very different from the abstract class. Changing client code at that point to use the interface will be much more painful than designing client code to do so from the start. – supercat Jul 26 '18 at 20:58
  • 1
    @supercat I could agree assuming you're writing a library. If it's an application, then I can't see any problem replacing all occurrences at once. My Eclipse can do it (Java, not C#), but if it couldn't, it's just like `find ... -exec perl -pi -e ...` and possibly fixing trivial compile errors. My point is: *The advantage of not having a (currently) useless thing is much bigger than the possible future savings.* – maaartinus Jul 26 '18 at 22:11
  • The first sentence would be correct, if you marked up `interface` as code (you are talking about C#-`interface`s, not an abstract interface, like any set of classes/functions/etc), and ended it "... but still outlaw full multiple inheritance." There's no need for `interface`s if you have MI. – Deduplicator Jul 27 '18 at 07:27
  • After revisiting this issue - I personally think about what I am going to be coding before I code it. If I know I want to use an API, I will write the interface before anything else, and that is the only way I will access implementations of it - via the interface. This allows you to provide polymorphism to classes that you want to access in different ways, without adding overhead where not needed. It's very straightforward - if the API is important and will have multiple implementations, then its Interfaces first, abstract classes second, concrete implementations last. – Rodney P. Barbati Mar 06 '19 at 03:03
0

If we want to explicitly answer the question, author says "Does it make sense to define this interface if I already have an abstract class TypeNameMapper or abstract class is just enough?"

The answer is yes and no - Yes, you should create the interface even though you already have the abstract base class (because you should not be referring to the abstract base class in any client code), and no, because you should not have created the abstract base class in the absence of an interface.

That you haven't put enough thought into the API you are trying to build is evident. Come up with the API first, then provide a partial implementation if desired. The fact that your abstract base class does type checking in code is enough to tell you it isn't the right abstraction.

As in most things in OOD, when you are in the groove and have your object model done well, your code will inform you as to what comes next. Start with an interface, implement that interface in the classes you need it. If you find yourself writing similar code, extract it into an abstract base class - it it the interface that is important, the abstract base class is just a helper and there may be more than one.

Rodney P. Barbati
  • 197
  • 4
-1

This is quite difficult to answer without knowing the rest of the application.

Assuming you're using some kind of DI model and have designed your code to be as extendable as possible (so you can add new TypeNameMapper easily) I would say yes.

Reasons for:

  • You effectively get it for free, as the base class implements the interface you don't need to worry about it in any child implementations
  • Most IoC frameworks and Mocking libraries expect interfaces. While many of them do work with abstract classes it's not always a given and I'm a big believer in following the same path as the other 99% of a library's users where possible.

Reasons against:

  • Strictly speaking (as you've pointed out) you don't REALLY need to
  • If the class/interface changes there's a little additional overhead

All things considered I would say that you should create the interface. The reasons against are fairly negligible but, while possible to use abstracts in mocking and IoC it's often far easier with interfaces. Ultimately though, I wouldn't criticise a colleague's code if they went the other way.

Deduplicator
  • 8,591
  • 5
  • 31
  • 50
Liath
  • 3,406
  • 1
  • 21
  • 33