Dave inherited a data management tool. It was an antique WinForms application that users would use to edit a whole pile of business specific data in one of those awkward "we implemented a tree structure on top of an RDBMS" patterns. As users made changes, their edits would get marked with a "pending" status, allowing them to be saved in the database and seen by other users, but with a clear "this isn't for real yet" signal.
One developer had a simple task: update a text box with the number of pending changes, and if it's non-zero, make the text red and boldfaced. This developer knew that some of these data access methods might not return any data, so they were very careful to "handle" exceptions.
int changes = 0;
try
{
changes = DataNode.DataNodeDataSet(Convert.ToInt32(Status.PendingNew)).Tables[0].Rows.Count;
}
finally{}
try
{
changes += DataVersion.GetVersionTable(Convert.ToInt32(Status.PendingNew)).Rows.Count;
}
finally{}
try
{
changes += DataOrderVersion.DataOrderVersionDataSet(Convert.ToInt32(Status.PendingNew)).Tables[0].Rows.Count;
}
finally{}
if (changes > 0)
{
Pending.Font.Bold = true;
Pending.ForeColor = System.Drawing.Color.Red;
}
Pending.Text = changes.ToString();
The indentation is this way in the original source.
This… works. They set the changes variable to zero, then wrap all the attempts to access potentially null values in try blocks. In lieu of having an empty catch, which I suspect their linter was set to complain about, they opted to have an empty finally.
It works, and I hate it. Worse, while tossing exceptions is less efficient than doing a null check, this code probably doesn't execute in a context where that performance difference matters, so I'm having a hard time saying this is absolutely wrong. And that just makes me hate it more. This will conceal exceptions other than null references, so I guess that's a pretty valid concern, but that just seems like such a milquetoast complaint compared to my visceral reaction to this code.
And then there's the seemingly random indentation. Visual Studio automatically corrects the indentation for you! You have to work hard to get it messed up this badly!
In any case, I definitely have some pending changes for this code.
[Advertisement]
BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!
This post originally appeared on The Daily WTF.