public interface introduced in derived class

Question

Two classes, D1 and D2 derive from an abstract base class B. Each of them share common public interface declared in B but each of them might also have their own specific public interface (e.g. D2 has D2.Bar() which makes sense only for D2 objects):

public abstract class B
{
    public int N { get; set; }
    public abstract void Foo();
}

public class D1 : B
{
    public override void Foo()
    {            
    }
}

public class D2 : B
{
    public override void Foo()
    {
    }

    public void Bar()
    {            
    }
}

I keep mix of derived objects in a single collection, (e.g. list) as sometimes I have to call common (inherited) methods on all objects from the collection but sometimes I want to call Bar() on D2 objects only:

        var list = new List<B>();
        list.Add(new D1());
        list.Add(new D2());
        foreach(var b in list)
            if(b is D2)
                (b as D2).Bar();

I feel the code smell here. Downcasting is a bad idea, making decisions based on type checks is bad idea. If I move Bar() to base class, it will not make sense calling it on D1 objects (what would implementation of D1.Bar() contain?). Interface and composition don't help either. I feel this is a very common situation and am wondering what's the best practice in this case? How to avoid downcasting but allow calling public methods specific for derived types?

score 5 · Answer 1 · answered Aug 16 '13 at 13:44

5

It sounds to me like actually the "check and downcast" is precisely appropriate given your description:

sometimes I want to call Bar() on D2 objects only:

Now it's a bit of an odd requirement, but if it is a requirement, I think it's reasonable to implement it in a straightforward fashion rather than adding no-op implementations for operations which don't make sense on the base class.

However, I'd go about it slightly differently:

foreach (var d2 in list.OfType<D2>())
{
    d2.Bar();
}

Now that says exactly what you mean :)

answered Aug 16 '13 at 13:44

Jon Skeet

1,421,763
867
9,128
9,194

`B` used to be huge class representing two logical types of the entity. Both types were sharing some functionality but some functionality was applicable only for one or the other type so I decided to break `B` into sub-classes. – Bojan Komazec Aug 16 '13 at 13:57
@BojanKomazec: Okay... well it seems at least *potentially* reasonable to segregate the list like this. – Jon Skeet Aug 16 '13 at 13:58
Looks nice, but it doesn't avoid what the author was worrying about - it does the same "if-particular-subtype-then-downcast" logic as the original code, just in a disguised manner. – BartoszKP Aug 18 '13 at 17:42
@BartoszKP: I never suggested it *did* do anything else. I don't think it "disguises" it - it just expresses it more cleanly, IMO. I explicitly said in the answer that I think it's okay to do this kind of thing sometimes, when it expresses the requirement simply. – Jon Skeet Aug 18 '13 at 17:53
Yes, you're right, you've pointed that out clearly. I don't agree that this is a good way to express this requirement, but on this abstraction level it's perhaps a very subjective matter :) – BartoszKP Aug 18 '13 at 17:57
@BartoszKP: We don't have enough context to say, basically. Sometimes it would make sense to create separate interfaces - sometimes it wouldn't. I did say it was an odd requirement. I don't think it's a good idea to be dogmatic about this sort of thing - a balance of idealism and pragmatism is useful. – Jon Skeet Aug 18 '13 at 17:58

score 1 · Answer 2 · answered Aug 16 '13 at 14:11

Seems like D2 is adding more behaviour to B than defined initially. This suggest that Single Responsibility Principle is being violated (D2 is doing more than one thing). The downcast suggests also that it does not necessarily makes sense to hold D1 and D2 in the same list. So, maybe "IS A" relationship is not appropriate here? I'd try to switch to composition and use a set of precisely defined interfaces for D1 and D2, referring to Interface Segregation Principle. Then probably D1 and D2 could be mixed in a list, but only if you are interested in one particular behaviour (interface), and for another interface you would have only D2's in the list (without knowing it, nor caring about it).

public interface introduced in derived class

2 Answers2