PJSen Blog

Recently I have spotted the following method in the large C# code base:

This code works and does what it supposed to do. However, I had a slight inconvenience while debugging it. I tend to frequently use Visual Studio DEBUG->Exceptions->CLR Exceptions Thrown (check this out!) functionality which to me is invaluable tool for diagnosing actual source of an exception. The code base relied heavily on this very ConvToInt method, thus it generated lots of exceptions and caused Visual Studio to break in with the debugger over and over again. I then had to disable CLR Exceptions Thrown to protect myself from being hit by flying exceptions all the time. Having switched this off I ended up with somehow incomplete diagnosing capabilities. It is bad either way. So, what I did was basically simple refactoring:

This method also works. One can even argue for better performance of this code, because throwing exceptions is considered to be slow. And this is also correct. Although performance was not key factor here (for line of business applications rarely is), but I measured it anyway. I ran both methods in a for loop 5 million times in release mode having wrapped them with appropriate calls to the methods of Stopwatch class. The results are surely not surprising. For valid string representations of a number, the former method (i.e. one using System.Convert) gave the average result of

663 milliseconds

and the latter (i.e. one using TryParse) gave the average result of

642 milliseconds

We can safely assume both methods have the same performance in this case. Then I ran the test with a not valid string representation of a number (i.e. passing “x” as an argument). Now the TryParse version gave the average result of:

546 milliseconds

And the System.Convert version, which indeed repeatedly threw exceptions gave the (not average, I ran this once) result of

233739 milliseconds

That is a huge difference in 3 orders of magnitude. Then I was fairly convinced my small and undoubtedly not impressive refactoring was right and justified. Except that it is not correct. It has worked well and has been successfully tested. But after a few weeks, when a different set of use cases was being tested, the application called ConvToInt with -1 in the second argument. It turned out, that the method returned 0, not the -1 for invalid string representations of a number. What I want to convey here is:

TryParse sets its out argument to 0, even if it returns false and did not successfully convert a string value to a number.

I scanned the code base and have found this pattern a few times. Apparently I was not the only programmer to not know this little fact about TryParse method. Of course, it is well documented (http://msdn.microsoft.com/en-us/library/f02979c7.aspx). The problem with this very API to me seems even more serious. The 0 value is supposed to be the most frequently used number value when it comes to string conversion failure in general. However, in a construct like this above, it comes from TryParse itself, despite the fact that it is provided by the caller and, more importantly, is primarily expected to be used as a default number value in case of failure. One can easily get into trouble when he or she expects (and passes as argument) different default value, e.g. -1 and still receives 0 because TryParse works this way by definition. Obviously the solution here is to add an if statement:

The performance does not get significantly worse because of this one conditional statement, I measured it and it is roughly the same.

The lessons learned here:

• Exceptions actually ARE EXPENSIVE. This is NOT a myth.
• Do not rely on the value passed as out variable to TryParse method in case of a failure. Always back up yourself with an if statement and check for the failure.
• More general one: learn the APIs, go to the documentation, do not simply assume you know what the method does. Even if it comes to basics. The descriptive and somewhat verbose method name can still turn into an evil when ran under edge cases. Always be 100% sure about what is the contract between the API authors and you, i.e. what the API actually does.