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") [...snip...]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.