DaedTech

Stories about Software

By

Casting is a Polymorphism Fail

Have you ever seen code that looked like the snippet here?

public class Menagerie
{
    private List _animals = new List();

    public void AddAnimal(Animal animal)
    {
        _animals.Add(animal);
    }

    public void MakeNoise()
    {
        foreach (var animal in _animals)
        {
            if (animal is Cat)
                ((Cat)animal).Meow();
            else if (animal is Dog)
                ((Dog)animal).Bark();
        }
    }
}

You probably have seen code like this, and I hope that it makes you sad. I know it makes me sad. It makes me sad because it’s clearly the result of a fundamental failure to understand (or at least implement) polymorphism. Code written like this follows an inheritance structure, but it completely misses the point of that structure, which is the ability to do this instead:

public class Menagerie
{
    private List _animals = new List();

    public void AddAnimal(Animal animal)
    {
        _animals.Add(animal);
    }

    public void MakeNoise()
    {
        foreach (var animal in _animals)
            animal.MakeNoise();
    }
}

What’s so great about this? Well, consider what happens if I want to add “Bird” or “Bear” to the mix. In the first example with casting, I have to add a class for my new animal, and then I have to crack open the menagerie class and add code to the MakeNoise() method that figures out how to tell my new animal to make noise. In the second example, I simply have to add the class and override the base class’s MakeNoise() method and Menagerie will ‘magically’ work without any source code changes. This is a powerful step toward the open/closed principle and the real spirit of polymorphism — the ability to add functionality to a system with a minimum amount of upheaval.

But what about more subtle instances of casting? Take the iconic:

public void HandleButtonClicked(object sender, EventArgs e)
{
    var button = (Button)sender;
    button.Content = "I was clicked!";
}

Is this a polymorphism failure? It can’t be, can it? I mean, this is the pattern for event subscription/handling laid out by Microsoft in the C# programming guide. Surely those guys know what they’re doing.

As a matter of fact, I firmly believe that they do know what they’re doing, but I also believe that this pattern was conceived of and created many moons ago, before the language had some of the constructs that it currently does (like generics and various frameworks) and followed some of the patterns that it currently does. I can’t claim with any authority that the designers of this pattern would ask for a mulligan knowing what they do now, but I can say that patterns like this, especially ones that become near-universal conventions, tend to build up quite a head of steam. That is to say, if we suddenly started writing even handlers with strongly typed senders, a lot of event producing code simply wouldn’t work with what we were doing.

So I contend that it is a polymorphism failure and that casting, in general, should be avoided as much as possible. However, I feel odd going against a Microsoft standard in a language designed by Microsoft. Let’s bring in an expert on the matter. Eric Lippert, principal developer on the C# compiler team, had this to say in a stack overflow post:

Both kinds of casts are red flags. The first kind of cast raises the question “why exactly is it that the developer knows something that the compiler doesn’t?” If you are in that situation then the better thing to do is usually to change the program so that the compiler does have a handle on reality. Then you don’t need the cast; the analysis is done at compile time.

The “first kind” of cast he’s referring to is one he defines earlier in his post as one where the developer “[knows] the runtime type of this expression but the compiler does not know it.” That is the kind that I’m discussing here, which is why I chose that specific portion of his post. In our case, the developer knows that “sender” is a button but the compiler does not know that. Eric’s point, and one with which I wholeheartedly agree, is “why doesn’t the compiler know it and why don’t we do our best to make that happen?” It just seems like a bad idea to run a reality deficit between yourself and the compiler as you go. I mean, I know that the sender is a button. You know the sender is a button. The method knows the sender is a button (if we take its name, containing “ButtonClicked” at face value). Maintainers know the sender is a button. Why does everyone know sender is a button except for the compiler, who has to be explicitly and awkwardly informed in spite of being the most knowledgeable and important party in this whole situation?

But I roll all of this into a broader point about a polymorphic approach in general. If we think of types as hierarchical (inheritance) or composed (interface implementation), then there’s some exact type that suits my needs. There may be more than one, but there will be a best one. When writing a method and accepting parameters, I should accept as general a type as possible without needing to cast so that I can be of the most service. When returning something, I should be as specific as possible to give clients the most options. But when I talk about “possible” I’m talking about not casting.

If I start casting, I introduce error possibilities, but I also necessarily introduce a situation where I’m treating an object as two different things in the same scope. This isn’t just jarring from a readability perspective — it’s a maintenance problem. Polymorphism allows me to care only about some public interface specification and not implementation details — as long as the thing I get has the public API I need, I don’t really care about any details. But as soon as I have to understand enough about an object to understand that it’s actually a different object masquerading as the one I want, polymorphism is right out the window and I suddenly depend on knowing the intricate relationship details of the class in question. Now I break not only if my direct collaborators change, but also if some inheritance hierarchy or interface hierarchy I’m not even aware of changes.

The reason I’m posting all of this isn’t to suggest that casting should never happen. Clearly sometimes it’s necessary, particularly if it’s forced on you by some API or framework. My hope though is that you’ll look at it with more suspicion — as a “red flag”, in the words of Eric Lippert. Are you casting because it’s forced on you by external factors, or are you casting to communicate with the compiler? Because if it’s the latter, there are other, better ways to achieve the desired effect that will leave your code more elegant, understandable, and maintainable.

By the way, if you liked this post and you're new here, check out this page as a good place to start for more content that you might enjoy.
34 Comments
Oldest
Newest Most Voted
Inline Feedbacks
View all comments
Mike Branstein
Mike Branstein
11 years ago

Thanks, Erik! In the button clicked example, how would you propose avoiding the cast? What would be the more appropriate pattern?

Erik Dietrich
11 years ago
Reply to  Mike Branstein

Personally, I wouldn’t avoid the cast in this situation because the consequences are too steep for the benefit to be worth it. The cast is awkward but strongly typing the handler would make it unusable as a handler in frameworks like WPF. Your code literally won’t compile and that would necessitate some really awkward work-arounds that would be jarring to maintainers or anyone else working with you. My point in this article was less to advocate against using the handlers with sender of type object and more to point out that just because it’s common to see that doesn’t mean… Read more »

JamesM
JamesM
11 years ago

Excellent post. Succintly and convincingly argued! Eric’s phrasing will stick in my head 🙂

Erik Dietrich
11 years ago
Reply to  JamesM

Thanks, and thanks for reading. Glad you liked the post 🙂

Chris Marisic
11 years ago

I once interviewed a developer who had 18 years of experience. I asked him what his biggest achievement was and talked about this project saying how it supported all of these different things polymorphically. So i press him on it for more details of explaining polymorphism and he defines it (and how his app did it) as specifically requiring casting. Explaining polymorphism by directly violating the Liskov Substitution Principle is definitely a clear way to **never** get hired by me.

Actually after posting this comment, i noticed one major shortcoming in this article, your lack of mentioning the LSP.

Erik Dietrich
11 years ago
Reply to  Chris Marisic

lol.. “jump table polymorphism” is how I’ve come to think of that sort of ‘pattern’. I always feel that if you pointed out to someone like your interviewee that he could just have a single class with an enum property called “ObjectType” for clients of his class to switch over he’d say “hey yeah — that’s a great idea!” And yeah, good point about LSP. Reading your comment, my reaction was “really, I never mentioned that?!” and now I see that I sure didn’t. I’ve touched on SOLID’s S and O in recent posts, but completely whiffed on an opportunity… Read more »

John Atten
11 years ago
Reply to  Erik Dietrich

Also funny! (single class with “ObjectType” property).

John Atten
11 years ago
Reply to  Chris Marisic

Funny! I am self-taught, but when I “discovered” LSP I felt I finally I had made an important discovery. I’m no mathematician, and have no formal training in CS, but I was happy to learn that I *almost* had it right intuitively.

John Atten
11 years ago

Completely agree. For me, when I find myself typing a Cast statement, I most often find it’s time a re-evaluate something about my design and/or class structure. The Event Sourcing standard in .net generally (and C# specifically) poses challenges in this regard, but is can easily be overcome, if necessary, by defying convention and creating your own delegate structure. Does anyone have additional info on the original thinking behind the Hendlername(object sender, EventArgs e) convention? For the most part, I do my best to make the required properties necessary to properly handle an event available within the e argument by… Read more »

Erik Dietrich
11 years ago
Reply to  John Atten

This is an interesting thing to mull over, and I don’t really have an answer off the top. I’ve made a draft of it and added it to my list of future blog posts, but I’ll have to put in some research and prototyping before I have anything of much substance to say. If you (or anyone else reading) run across a particularly elegant solution — particularly one that is compatible with common practice somehow — I’d be very interested to hear.

John Atten
11 years ago
Reply to  Erik Dietrich

My “off the top of my head” additional thoughts are such (I have no formal schooling here, so I could be about to show some ignorance!): 1. Could it be said that if an object is subscribed to an event, then in theory, it should not be the source of the event which is of primary interest to the subscriber, but instead the affects? In other words, in most but not all cases, might the argument be that if I am awaiting notification that (for example) a certain button was clicked, I am less interested in the certain button, but… Read more »

Erik Dietrich
11 years ago
Reply to  John Atten

(1) is an example of a powerful OOP concept called “tell, don’t ask”. The goal is to eliminating querying objects about their state and making decisions (procedural style) in favor of telling objects you want them to do things and having them figure this out. The event source is your “command” and your object strives to do things with its internal state only without picking through the state of the sender. It’s a good thought. (2) is interesting, but requires a strong coupling between the presentation layer class (probably code behind) and the view. If you’re using a presentation layer… Read more »

John Atten
11 years ago
Reply to  Erik Dietrich

Your last thought is also a good one. which I employ when necessary, not just limited to event handling or casting. If I find I have a piece of stanky code which can’t be eliminated (at least, in the near-term) I at least do my best to constrain it to a single place. Good to know I have learned a few things from all this!

Ken
Ken
11 years ago
Reply to  Erik Dietrich

At one point I didn’t have an IDE to write C# so I used notepad. Tried to figure out how to send events from a TextBox sender. With the amount of work needed, I gave up in disgust. I did end up with one “leave” processing event for 9 of 47 boxes in a 7X7 grid. Every textbox was assigned a name with 2 fixed characters and 2 characters that identified the first and 2 characters that gave the decimal index of the box the user left (11,13,15,31…55) The other 40 always acted like labels, but these 9 were treated… Read more »

Erik Dietrich
11 years ago
Reply to  Ken

I’m having a bit of trouble picturing this — I’d probably have to see code snippet or two in order to offer an opinion if it constituted a “tell, don’t ask” paradigm. If you want to throw it up on gist, I’m happy to take a look and offer my thoughts.

Schmulik Raskin
Schmulik Raskin
11 years ago
Reply to  Erik Dietrich

I may be oversimplifying things, but in the case of .NET events there is a common delegate, EventHandler/EventHandler(T), which actually lives in the ‘System’ namespace in MSCorLib. This is where the ‘object sender’ comes from.

Andrew
Andrew
11 years ago
Reply to  Erik Dietrich

Maybe having a dynamic sender would solve the problem of having to actually type the cast out in this case.

Gary Wheeler
Gary Wheeler
11 years ago

WPF does not preclude having a single event handler that services multiple events from disparate controls. For example, the same event handler could address click events for a button, a combo box, and an image. You can argue that it is better form to have strongly-typed event handlers that call a common underlying method in this case, of course.

Erik Dietrich
11 years ago
Reply to  Gary Wheeler

This seems to be along the lines of the dispatcher I was alluding to in another comment, which is a good way to localize and isolate the casting. There might even be frameworks/toolkits that do this kind of thing for you (I seem to remember some along the lines of MVVM helpers).

thereverand
thereverand
11 years ago

Well done!

Erik Dietrich
11 years ago
Reply to  thereverand

Thanks! Glad you enjoyed the post.

Just Curious
Just Curious
11 years ago

How would you implement a visitor pattern?

Erik Dietrich
11 years ago
Reply to  Just Curious

Given that the concept of double dispatch (polymorphic dispatch on both visitor and element) is at the core of the pattern, I don’t think the cast is really avoidable from a practical perspective. One of the base will at least have to know about the others’ derived classes no matter what or there would be no useful functionality. I personally would have the abstract visitor base class use generics to cast to the visited/element type so that at least the concrete visitors didn’t all have to do it. Good point, though. I’d say this design pattern (and more generally, double… Read more »

Mark
Mark
11 years ago

With Silverlight and WPF, you could just use a Command binding and not HAVE an event on button click. That eliminates any need for casting sender entirely.

Erik Dietrich
11 years ago
Reply to  Mark

That really only works for a handful of controls, like Button and menu items. If I have a combo box and I’m interested in selected item changing, we’re right back at square one absent an MVVM framework that handles this sort of thing. (And yes, I realize that this could be addressed in the VM through binding, but there will always be some example of somewhere or another that someone needs an even handler that takes a sender.) However, I definitely like the way you think. Back it up a level and say “instead of banging our heads against a… Read more »

david neckels
david neckels
11 years ago

I don’t know. I mean, given your example, one would have to create a base animal class and new instances for every subclass. This is the typical inheriteance nightmare, the overloaded interfaces, the code bloat due to repeated interfaces. The switch statement is much shorter and localizes the code. It is better. It doesn’t scale, though. So the question to ask is really “how many animals can I expect?”. And/Or is this code purely internal or meant to be extendable? There are times when the switch statement beats bloated inheritance any day. Inhertance has seen its day and people now… Read more »

Erik Dietrich
11 years ago
Reply to  david neckels

This is how I look at it, personally. I could agree that for a snapshot at this moment in time, assuming that their were only two types of animals, the best approach might be something that didn’t involve animal classes at all and simply had two methods AddCat() and AddDog(), and then a simple if statement when iterating. This would be the least code at any rate and perhaps most understandable at a glance, particularly for anyone who was more accustomed to or favored a procedural coding style. You astutely point out that this doesn’t scale, but if you’re sure… Read more »

david neckels
david neckels
11 years ago
Reply to  Erik Dietrich

Well, I don’t disagree with you, but at the same time I find that OOP sometimes adds a lot of static when the class that are abstracted are too small. Kind of like gas station food, all wrapper, little food.
I think we both definitely are in agreement for the more complex case, though…

Erik Dietrich
11 years ago
Reply to  david neckels

Incidentally, I love the “gas station food” metaphor… lol.

trackback

[…] Casting is a Polymorphism Fail […]

trackback

[…] I’ve previously blogged that casting is a failure of polymorphism. And this is like casting on steroids and some […]

trackback

[…] that are interested in Dog to take an Animal and cast it? I’ve blogged previously about what I think of casting. Be specific. If your clients want it to be an animal, they can just declare the variable to which […]

dave falkner
dave falkner
10 years ago

Sometimes when a new product feature is suggested in a meeting, I like to joke, “Easy! We can just add a new enum value and then go support it everywhere!”

I have a specially-crafted interview question designed to probe a candidate’s level of understanding of this concept.

Erik Dietrich
10 years ago
Reply to  dave falkner

Something about that evokes the image of you weeding out “work harder, not smarter” candidates. I’ve definitely encountered a set of people that seem to love rolling up their sleeves and engaging in shotgun surgery throughout the code base. They confuse activity with productivity.