Casting is a Polymorphism Fail

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

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

    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<Animal> _animals = new List<Animal>();

    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.

  • Mike Branstein

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

  • JamesM

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

  • http://twitter.com/dotnetchris Chris Marisic

    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.

  • http://Typecastexception.com John Atten

    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 deriving from the EventArgs base. Agiain, it always feels like, if I need to perform that cast on sender to a specific type, quite possibly I need to re-examine my design.

  • http://www.daedtech.com/blog Erik Dietrich

    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 it’s a pattern that should be repeated. It’s kind of like saying that just because people might grab fast food in a pinch in the airport from time to time doesn’t mean that it’s a good idea to make every meal a shake, burger and fries. I think it’s important to recognize that casting is a red flag, even if you see prominent examples of it.

  • http://www.daedtech.com/blog Erik Dietrich

    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.

  • http://www.daedtech.com/blog Erik Dietrich

    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 to mention “L”, which is probably the one that I have fewest occasions to mention (and which is probably most interesting to me given my math background and its relationship to Hoare logic and verification). Good catch.

  • http://www.daedtech.com/blog Erik Dietrich

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

  • http://Typecastexception.com John Atten

    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.

  • http://Typecastexception.com John Atten

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

  • http://Typecastexception.com John Atten

    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 instead what it is I am supposed to do when that happens?

    2. Most (but not all) of the time, in order to subscribe to an event I tend to already have a reference of some sort to the object which is sourcing the event. For example, the button click event on a form. I would subscribe within the form to the button click event, which might then fire another event defined upon the form itself (i.e. “DataUpdated”) to which my client code has subscribed in the manner of Form1.DataSaved += Etc. In this paradigm, Form1 may care which button was clicked and perform some additional handling (validation or what have you) and *may* need to do something with the button itself. But Form1 already holds a reference to the button, and this does not need to cast the object argument of the event args. The client code most likely doesn’t need to know which control on Form1 initiated the “DataUpdated event, but will likely refer to an instance of Form1 itself, and be able to process accordingly. Of course, this breaks down if more than one control fires the same event *and* additional processing of the control is required. But this is where, if I really wanted to avoid casting, I might use a custom eventArgs which provides a reference to the control (or better yet, the property of the control) which requires processing.

    3. Sometimes, if I need the specific sender, I just get lazy (or don’t want to clutter up my project with a bunch of custom event Argument def’s) and do the cast!

    4. I could have this *all wrong* in which case, back to research!

  • http://www.daedtech.com/blog Erik Dietrich

    (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 pattern other than MVC (such as MVVM), this becomes a non-starter because the event handlers won’t have any knowledge of the UI controls on the form/control/page.

    I had another thought too, which is that if you can’t eliminate the problem, you can isolate it. In concept for instance, you could wire all of your events to the same weakly typed handler and have that handler dispatch/route to strongly typed ones that you supply. It’s a “fall on the grenade” kind of approach where you’re just isolating and segregating the undesirable code, but perhaps something to think further on to see if it’d be worth at least a prototype.

  • http://Typecastexception.com John Atten

    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!

  • Schmulik Raskin

    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.

  • Gary Wheeler

    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.

  • thereverand

    Well done!

  • Just Curious

    How would you implement a visitor pattern?

  • Mark

    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.

  • http://www.daedtech.com/blog Erik Dietrich

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

  • http://www.daedtech.com/blog Erik Dietrich

    Thanks! Glad you enjoyed the post.

  • http://www.daedtech.com/blog Erik Dietrich

    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 dispatch) falls into the same category as the event handling cast — not ideal but not practically avoidable if you’re going to use this technique. I personally tend to gravitate away from double dispatch/visitor without necessarily giving a lot of thought as to why, so I suppose I have the same taste as myself. All things are a matter of taste and tradeoffs, so this is not an indictment of the Visitor pattern or anything — just my personal taste.

  • http://www.daedtech.com/blog Erik Dietrich

    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 wall solving this problem, let’s see if we even need a solution right now.”

  • david neckels

    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 realize that, like any cure all, it has its plusses and minuses.

  • Andrew

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

  • Ken

    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 as either labels or editable textboxes. The value meaning of these boxes changed depending on what option was picked I’d have the event routine figure out which option was picked and send the location to 1 of two routines and exit.
    In any case the location of the box was crutial in properly assigning values in the code, the name was the easiest way I could think of to do it. Is this an example of tell, don’t ask processing?
    That’s a little different than a button which generally does one thing. Event processing usually creates custom events per button (My app had 1 button, its text changed from start to stop processing depending on the state of the code, so even with 1 custom event code, that was different too.)

  • http://www.daedtech.com/blog Erik Dietrich

    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 you don’t need it to, and won’t *ever* need it to, I suppose the argument could be made that it doesn’t hurt anything…

    But it’s not one that I would make, personally. My opinion is that C#, Java, and other pure OOP languages are just that — OOP languages. Switching over some primitive type (ala op code) is inherently procedural and you’re going against the grain when you do things like that in an OOP language, just as you’d be going against the grain to have a series of function pointers and simulated polymorphism tricks to do this in C. In C#, types are the basic currency of the language, so I might as well use them. And, in terms of segregating responsibilities, I don’t want my Menagerie class worrying about what noises animals make — I want it worrying about how to group and collect animals. Let the individual animal classes worry about individual animal concerns.

    On balance, I agree that “bloated” inheritance is a problem and a source of needless complexity (I think code starts to smell at the third derivation and grows from there, personally), but I wouldn’t view a simple “IsA” relationship of various kinds of animals to a base class Animal as bloated. I think that’s a better way to decouple the logic of aggregating animals from the logic of animals making noises or eating or whatever it is that we’re modeling.

    (I do like the YAGNI argument, though, tends to keep things honest)

  • http://www.daedtech.com/blog Erik Dietrich

    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.

  • david neckels

    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…

  • http://www.daedtech.com/blog Erik Dietrich

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

  • Pingback: A Blog Grows Up: DaedTech Year in Review for 2012 | DaedTech

  • Pingback: Faking An Interface Mapping in Entity Framework | DaedTech

  • Pingback: What To Return: IEnumerable or IList? | DaedTech

  • dave falkner

    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.

  • http://www.daedtech.com/blog Erik Dietrich

    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.