CODEWITHSUNDEEP
c#oopinheritancedowncast
Bojan Komazec
Bojan Komazec

Reputation: 9536

public interface introduced in derived class

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?

Upvotes: 1

Views: 97

Answers (2)

BartoszKP
BartoszKP

Reputation: 35901

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).

Upvotes: 1

Jon Skeet
Jon Skeet

Reputation: 1502985

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 :)

Upvotes: 5

Related Questions