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.