CodeSOD: Two Comparisons, Hold the Case

There are a lot of times when we want string comparisons to be case insensitive. It's quite a lot of cases, so every language is going to give us a way to easily specify that's what we want.

Take, for example, this C# code, written by one of Robin's team-mates.

override public void VerifyConnectionDetails(bool queryRequired){ if (this.Name.EndsWith("2", StringComparison.OrdinalIgnoreCase) || this.Name.EndsWith("2", StringComparison.OrdinalIgnoreCase)) { // Let some time pass to simulate PIDS behavior System.Threading.Thread.Sleep(100); IsConnected = false; } else { IsConnected = true; } IsConnected = true;}

Here, we want to have two different code paths if the Name ends in "2". But we don't want one of those sneaky lower-case ₂s to throw things off, so we make this a case insensitive comparison.

Which, honestly, it's a perfectly reasonable thing to do. It may not have always been a "2" that they were looking for, so a case insensitive check may have made more sense in the past. But then... why do the check twice?

And this is where the flow of code drifts from "silly" to just weird. If we're on the "2" code path, we pause for 100ms and then set IsConnected to false. Otherwise, we set it to true. Then no matter what, we set it to true.

I suspect the "2" code path meant to have the false set before the sleep, to simulate checking a connection. Then it sets it to true, simulating that the connection has been established. But I don't know that from this code, instead what I see is a really weird way to force an IsConnected property to be true.

[Advertisement] ProGet's got you covered with security and access controls on your NuGet feeds. Learn more.