On the verge of evil

The WebForms APIs in ASP .Net tried to make web programming look like native programming – you designed pages in a WYSIWYG editor and bound event handlers to controls and pretended the request/response cycle didn’t exist. It was a bit of a mess.

One way it was messy was that you now had two different approaches to styling elements in your UI. Your could go the route of using CSS, like a normal web page. But every web Control object also exposed a bunch of properties that you can access directly from server-side code, allowing you to do things like myControl.BorderColor = Color.Red.

That’s all well and good, or good and bad, if we’re being honest, but that brings us to Leandrosubmission. As you search through the legacy code base, this one a small clip pops up:

protected void BorderColourSet(Control ctrl, Color col)

	switch (ctrl.GetType().Name.ToString())
	
		case "TextBox":
			((TextBox)ctrl).BorderColor = col;
			break;
		case "RadioButtonList":
			((RadioButtonList)ctrl).BorderColor = col;
			break;
		case "DropDownList" :
			((DropDownList)ctrl).BorderColor = col;
			break;
		case "CheckBox" :
			((CheckBox)ctrl).BorderColor = col;
			break;

Someone didn’t understand how inheritance and polymorphism work. BorderColor is a property Control, and is inherited in all other controls. So, if nothing else, the extra churn in each branch is unnecessary.

Now, there’s a world where this function does validation, making sure you’re only setting the border color on these specific controls – maybe they don’t want border color changing on buttons for example? If you pass something not in this list, the function does nothing. Even then, I’m sure there are more elegant ways to express that constraint.

Elegant ways that are irrelevant – they were not trying to express a limitation here. What led Leandro to this snippet was finding a bug where the buttons weren’t setting the border color correctly. It seems like the original developer wrote this without thinking about buttons.

And once you realize that this was only meant to be a practical wrapping function BorderColor property, you have a second realization that this function is useless. ctrl.BorderColor = col would achieve the same thing, simpler and clearer – which Leandro refactored the code for.