If anybody is writing .Net code that in the future I am going to have to find an error in, can they refrain from doing things like this:
catch (Exception e)
throw new ApplicationException("ConnectionException has occured: " + e.Message);
The particular application I was working on this morning used this particular gem pretty well everywhere. The problem was that we were getting an unhandled ApplicationException – the re-thrown error wasn’t being trapped anywhere else – and since the above bit of code throws away the stack trace from the original exception, we were left trying to guess where the error was actually occurring.
The irony is that the code was written by a contractor who apparently had a pretty stellar CV, and amongst other things quite regularly criticised other people on the team over their apparent bad programming. His contract wasn’t renewed, and since then we’ve been discovering quite how bad his programming really is – a definite lesson that however good a contractor appears on paper, you need to properly monitor what they are doing.
Other gems in this particular application include every SQL statement being built by concatenating strings together – and no, not a
StringBuilder in sight. Using
ToString() to do the conversions where needed, including quite a few points where he calls
ToString() on a string – indeed a large amount of the data is just held as strings anyway, only being converted back to integers or dates in the stored procedures when SQLServer needs the right types. He had also written his own replace function, which he was using instead of
String.Replace although it was functionally identical, and that wasn’t the only place he’d hand crafted code that replicated functionality in the framework. The code was littered with other common errors, for example it’s a good job we don’t have anybody with the surname O’Reilly…
Since we were fixing a specific problem, much as I would like to, there wasn’t the opportunity to fix all of these other problems, essentially we have to wait until something else breaks, and fix it then. And the source of the problem today? A stored procedure that was trying to convert one of the multitude of strings being passed through back to an integer in order to use it. That conversion was failing and throwing an error that was passed back to the client and helpfully chucked away by the error handling code I started with – the only clue we had as to where it was going wrong was that the error message seemed like it was coming from SQLServer.
Also published on Medium.