CodeSOD: A Long Conversion
Let's talk a little bit about .NET's TryParse method. Many types, especially the built in numerics, support it, alongside a Parse. The key difference between Parse and TryParse is that TryParse bakes the exception handling logic in it. Instead of using exceptions to tell you if it can parse or not, it returns a boolean value, instead.
If, for example, you wanted to take an input, and either store it as an integer in a database, or store a null, you might do something like this:
int result;if (int.TryParse(data, out result)) { rowData[column] = result;} else { rowData[column] = DBNull.Value;}
There are certainly better, cleaner ways to handle this. Russell F. has a co-worker that has a much uglier way to handle this.
private void BuildIntColumns(string data, DataRow rowData, int startIndex, int length, string columnName, FileInfo file, string tableName){ if (data.Trim().Length > startIndex) { try { int resultOut; if (data.Substring(startIndex, length).Trim() == "" || string.IsNullOrEmpty(data.Substring(startIndex, length).Trim())) { rowData[columnName] = DBNull.Value; } else if (int.TryParse(data.Substring(startIndex, length).Trim(), out resultOut) == false) { rowData[columnName] = DBNull.Value; } else { rowData[columnName] = Convert.ToInt32(data.Substring(startIndex, length).Trim()); } } catch (Exception e) { rowData[columnName] = DBNull.Value; SaveErrorData(file, data, e.Message, tableName); } }}private void BuildLongColumns(string data, DataRow rowData, int startIndex, int length, string columnName, FileInfo file, string tableName){ if (data.Trim().Length > startIndex) { try { int resultOut; if (data.Substring(startIndex, length).Trim() == "" || string.IsNullOrEmpty(data.Substring(startIndex, length).Trim())) { rowData[columnName] = DBNull.Value; } else if (int.TryParse(data.Substring(startIndex, length).Trim(), out resultOut) == false) { rowData[columnName] = DBNull.Value; } else { rowData[columnName] = Convert.ToInt64(data.Substring(startIndex, length).Trim()); } } catch (Exception e) { rowData[columnName] = DBNull.Value; SaveErrorData(file, data, e.Message, tableName); } }}
Here's a case where the developer knows that methods like int.TryParse and string.IsNullOrEmpty exist, but they don't understand them. More worrying, every operation has to be on a Substring for some reason, which implies that they're processing strings which contain multiple fields of data. Presumably that means there's a mainframe with fixed-width records someplace in the backend, but certainly splitting while converting falls is a bad practice.
For bonus points, compare the BuildIntColumns with BuildLongColumns. There's an extremely subtle bug in the BuildLongColumns- specifically, they still do an int.TryParse, but this isn't an int, it's a long. If you actually tried to feed it a long integer, it would consider it invalid.
Russell adds: "I found this in an old file - no clue who wrote it or how it came to exist (I guess I could go through the commit logs, but that sounds like a lot of work)."
[Advertisement] Forget logs. Next time you're struggling to replicate error, crash and performance issues in your apps - Think Raygun! Installs in minutes. Learn more.