I saw the following comment online the other day when searching for an answer to an unrelated question:

Why are people so in love with catch-blocks as flow control? What was it about ‘if’ statements that burned you people so badly, in the past?

if ( !collection.Contains(x))
{
    collection.Add(x);
}

vs

try
{
    collection.Add(x);
}
catch (ArgumentException)
{
    //...
}

The latter is more keystrokes at code-time, a lot more CPU cycles at run-time, and it gets in the way of debugging. Give it up, people!

I’d make the following comments in response:

  • The second construct is a better design;
  • The second construct has better thread safety; and
  • Code brevity is a poor goal on its own;

Let’s explore each of these points.

Better Design

One common precept for good OO design is “tell, don’t ask”. In other words, don’t treat your objects like dumb containers to be manipulated, but instead like active entities that participate in your application.

From this perspective, it is bad form to inquire about state and then turn around and change the object depending on that state.

For example, consider two work colleagues having a conversation about a Sales presentation - which of these examples is better?

Example One

Jane: Hey, John, am I on the list of people you’re inviting to that Sales presentation on the 14th?
John: No.
Jane: Why not? You know I asked you last week to include me. Change the list - I want to come.
John: Ok.

Example Two

Jane: Hey, John, would you make sure I’m included on the list of invitations to that Sales presentation on the 14th?
John: Sure, no problem.

Ok, so the examples are artificial - but they make the point. In the first example, John is being treated as some kind of inanimate object and Jane does everything herself; in the second, John is treated as a valued colleague and Jane doesn’t need to worry about whether she was already on the invitation list or not.

We see the same in the code - in the “If I’m not there, add me” case, the calling code has to worry about extra details that are really not relevant.

It would be better if the collection had an alternative to Add(), one that didn’t raise an exception if the item was already present.

// Add X if it's not already there
collection.Include(x);

Thread Safety

It’s naive in today’s development environments to assume a single-threaded development model. Every .NET application is threaded, by nature, because of garbage collection and other background infrastructure.

Unless you’re writing code directly on User Interfaces, you have to consider the possibility that someone, somewhere, will try to use your code in a multi-threaded environment.

In the “If I’m not there, add me” scenario, you run the risk that another thread updates the list behind your back, resulting in an exception even though the if statement should (at first blush, anyway) prevent them from occurring.

The “try, then catch” approach - or, indeed the alternative API suggested above - works to make the code more robust in the multi-threaded scenario.

Think of it as cheap insurance - pay a minor premium now and avoid a whole raft of potential headaches later on … subtle, hard to reproduce and nasty headaches, to boot.

It’s been my experience that multi-threading works best when each object takes care of its own locking issues - trying to handle the locks at too high a level seems to prevent the application from truly leveraging the possibilities of concurrent operation.

Code Brevity

Most code is of the “Write Once, Read Many times” (or WORM) variety - written once by a developer, and then read dozens (or hundreds) of the times by the colleagues of the original developer, trying to work out what the code actually does.

Because code is read so very many times more often than it is written, the ROI of writing well-formed code is very high. A few seconds or minutes additional effort up front can save other developers minutes or hours or days of additional work down the track.

Given this situation, any argument that deprecates a coding style simply because it is more verbose seems (to me) to be pretty tenuous.

In a lot of ways, it’s like Occam’s razor, itself often incorrectly quoted as “The simplest explanation is the best one”. The correct quote (from Albert Einstein) is “Theories should be as simple as possible, but no simpler”.

Code should be written to be as simple as possible - but no simpler. A more verbose construct should be discarded if it delivers no value, not just because it is verbose.

Comments

blog comments powered by Disqus