What is wrong with testing an object to see if it implements an interface?

Question

In the comments of this answer it is stated that "checking whether the object has implemented the interface , rampant as it may be, is a bad thing"

Below is what I believe is an example of this practice:

public interface IFoo {    void Bar(); }  public void DoSomething(IEnumerable<object> things) {    foreach(var o in things)    {        if(o is IFoo)            ((IFoo)o).Bar();    } } 

With my curiosity piqued as someone who has used variations of this pattern before, I searched for a good example or explanation of why it is a bad thing and was unable to find one.

While it is very possible that I misunderstood the comment, can someone provide me with an example or link to better explain the comment?

Answer 1

It depends on what you're trying to do. Sometimes it can be appropriate - examples could include:

• LINQ to Objects, where it's used to optimize operations like Count which can be performed more efficiently on an IList<T> via the specialized members.
• LINQ to XML, where it's used to provide a really friendly API which accepts a wide range of types, iterating over values where appropriate
• If you wanted to find all the controls of a certain type under a particular control in Windows Forms, you would want to check whether each control was a container to determine whether or not to recurse.

In other cases it's less appropriate and you should consider whether you can change the parameter type instead. It's definitely a "smell" - normally you shouldn't concern yourself with the implementation details of whatever has been handed to you; you should just use the API provided by the declared parameter type. This is also known as a violation of the Liskov Substitution Principle.

Whatever the dogmatic developers around may say, there are times when you simply do want to check an object's execution time type. It's hard to override object.Equals(object) correctly without using is/as/GetType, for example :) It's not always a bad thing, but it should always make you consider whether there's a better approach. Use sparingly, only where it's genuinely the most appropriate design.

---

I would personally rather write the code you've shown like this, mind you:

public void DoSomething(IEnumerable<object> things) {     foreach(var foo in things.OfType<IFoo>())     {         foo.Bar();     } } 

It accomplishes the same thing, but in a neater way :)

Answer 2

I would expect the method to look like this, it seems much safer:

public void DoSomething(IEnumerable<IFoo> things) {    foreach(var o in things)    {            o.Bar();    } } 

To read about the referred violation of the Liskov Principle: What is the Liskov Substitution Principle?