CodeSOD: Tied to the Train Tracks

Ah, the joys of stringly typed code. Everything can be turned into a string, so why not treat everything as a string? It certainly worked for Sebastian's co-worker, who also took it to another level:

If GetTrainAtStation(aTrainNo.ToString) Is Nothing Then iTheTrainAtStation.Add(String.Format("{0}", aTrainNo.ToString), aTrainAtStationItem)End If

Here, this Visual Basic .NET code wants to add an entry to a dictionary. The key is the train's number, and the value is a aTrainAtStationItem. Now, a train's number probably should be a string- even if the content is all digits, it's not actually a number for use in mathematics.

But it doesn't matter, since we turn it into a string right away to pass to GetTrainAtStation. That isn't string enough, though, as to turn it into a key, we not only ToString it, but also String.Format the string into the same string.

This block here is a representative sample. Sebastian writes:

Choosing the correct data type is sometimes a problem. When in doubt, my co-workers choose the good, old String. It can be null/nothing, which sometimes is handy. Everything can be .ToString():ed to it, clearly making it extra good.

Also, remember how I suggested that aTrainNo should be a string? Yeah, this was the one time they opted to not use a string, because they saw "number" in the name and decided to make it an integer.

There are plenty of other WTFs in this code that aren't part of this sample. That dictionary, iTheTrainAtStation, can be accessed via a getter- and that getter wraps a synclock around the operation. Note: mutations of the dictionary aren't wrapped in a synclock, making the thread safety of locking nonexistent.

Speaking of threading, this codebase also has a singleton implementation which isn't thread safe, which created race conditions where it was possible the singleton object to get double created.

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!