Fail Fast, and Mind the Outs

I had a learning opportunity on two coding concepts (in C#) yesterday:

• out parameters are tricksy and not to be trusted.
• Fail fast. When the world starts getting uncertain and hairy, throw an exception and get out of there.
  

The setup: I'm retrieving app configuration values from the database, a minimum and maximum allowable threshold. The db table is generic, containing key/value pairs for many purposes, so the values are stored as varchars, but I want integers here. If I don't get a value, I have some defaults we should use. The colleague who would be reviewing my code has declared he is allergic to nested if/else blocks, so I need a direct path through this logic.

Too crufty: One might be tempted to think about the logic this way. If I get a value from the database, but it isn't an int, set it to the default; if I don't get a value from the database, set it to the default. When you say it in English, you can hear the repeated code, violating the DRY principle. It's a small repetition, and in a very localized piece of code, but still icky.

Tripping over outs: Since I didn't want to repeat that default value, and I wasn't happy with those elses, I tried this. The default value is 100; if you get a different integer from the database, use that. But I didn't get it quite right:

[using a data reader]
int max = 100;
if (dataReader.Read())
{
   Int32.TryParse(dataReader.GetString(0), out max); 
     //This is wrong.
}
return max;

Can you see what I was thinking, though? "If you successfully parse the result into an integer, set 'max' to that value. Otherwise, leave it alone." That was my mistake. What actually happens is, if TryParse can't parse it into an integer, it makes it undefined. From an msdn article, "The value of an out argument will not be passed to the out parameter."

What I really needed to do there is, "if not int.tryparse, then set it to my default value." I should check the boolean return value whenever I use TryParse.

Hold on, cowboy: But what does it mean if the configuration value in my database isn't an integer? Somebody typed something screwy. And they probably did it by updating the database, not using the admin screen. The world is in an unknown state, society is breaking down—cats and dogs, living together...

I don't want to set any default value. I want to throw an exception and get the heck out of there. So the final version is:

[using a data reader]
int max = 100;
if (dataReader.Read())
{
   max = Int32.Parse(dataReader.GetString(0)); 
     //Fail fast and make your escape.
}
return max;

This achieves three design principles, which is why I wanted to write it down and reinforce my learning:

1. Find the most direct path through the logic. Elses are a smell.
2. Don't repeat yourself. Even though the repeating here was trivial, it caught my attention and made me think through what should really happen, which led me to...
3. When you start getting unexpected behavior, hiding it and swallowing it will cause worse problems. If you get a "that can't happen" scenario, throw an exception.