CodeSOD: Parents In Control

I've said many unkind things about ASP.Net WebForms in the past. While it leaves a lot to be desired as a web development environment, its event driven approach does map to a lot of folks understanding of software. Click a button, execute code in response. Change a radio button, execute code in response. A simple, easy to understand architecture.

Well, sometimes simple and easy to understand.

A number of years ago, Kyle was tasked with updating his employer's "benefits election" site for the new year, and found this:

Protected Sub rdoSmokeYes_CheckedChanged(sender As Object, e As System.EventArgs)
    Dim rdoEmployeeYes As RadioButton
    rdoEmployeeYes = CType(CType(sender, RadioButton).Parent, Panel).FindControl("rdoSmokeYes")
End Sub

This is an event handler. If the user altered the rdoSmokeYes radio button, this event handler will fire. The ASP.Net event handler takes two parameters: the sender (as an Object), aka the rdoSmokeYes radio button, and an EventArgs object, containing some details about the event itself.

I want to stress: sender contains the radio button which triggered this event.

Which is why the next two lines are so stupid. We create a variable to hold a RadioButton, and then we populate it. We populate it by converting the sender back into a radio button, finding its parent, converting its parent back into a Panel, then finding the control called rdoSmokeYes- the very control that triggered this event.

A simple rdoEmployeeYes = CType(sender, RadioButton) would do the same thing.

But this code contains another WTF. FindControl returns the type System.Web.UI.Control, not a RadioButton. Which means this is a narrowing conversion, which should be a compile time error. Except this is VB.Net, which means you can turn Option Strict Off, permitting such conversions.

And, as Kyle informs us, this is just an example snippet of a codebase that has huge piles of code very similar to this. But it's even worse- because this convention isn't followed consistently. Nothing about the code is consistent, actually, as the development was done over many years by many developers following no specific kind of standard.

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.

This post originally appeared on The Daily WTF.

Leave a Reply

Your email address will not be published. Required fields are marked *