A pet hate and anti-pattern: A method returning a bool that can never return false, but can throw an Exception.

Recently I came across some code that, while syntactically correct, does something that always gets to me.

Let’s illustrate it with an example:

using System;

namespace Example
{
    public class AntiPatterns
    {
        public bool NeverReturnsFalse()
        {
            try
            {
                // Do something complicated
                // that can fail and throw.
            }
            catch (Exception ex)
            {
                // Code that logs the Exception.

                throw;
            }

            return true;
        }
    }
}

Full disclosure: I didn’t study programming or computer science, but slipped into this industry by accident. I’m a self taught programmer. However, one must use one’s common sense…

Methods that return a bool should return either true or false. (I can’t believe I had to write that.) They represent actions attempted which may succeed or fail. The above method can never return false. Its return value is hard-coded as true, so it will only ever return true unless an error occurs, in which case it will throw an Exception.

Why return a bool at all here? It makes no sense because the operation attempted, whatever it may be, can’t possibly return false (although it can fail). That means whoever consumes the method is probably going to write code, either in an else block or maybe a conditional statement, that checks the return value of this method. That is, whoever consumes this method is going to waste their time writing code that can never run, or check a condition that doesn’t need checking, because unless they have access to this code – which they won’t if it is a library – they must assume that a method returning a bool can either succeed or fail.

A less serious issue is that it can throw an Exception, but this is only less serious because everybody should always catch Exceptions anyway. However, what they need to do in their Exception handler is now ambiguous. Should I treat an Exception like false? Is it a recoverable error? Who knows? If I’m calling it from a thread that’s not critical to my application, I’m going to have to abort whatever that thread does gracefully and just report that it failed (and hope that it might work next time?). If I’m calling it from the one and only thread of a desktop application, I have no choice but to crash the application because I really have no idea what this error means and it is thus unsafe to continue. And that’s not cool.

Just don’t do that.

What common sense dictates you do instead:

  1. If you’re going to return a bool, it must be possible for it to be true or false. Otherwise don’t return a bool. Return void.
  2. If you’re writing a library that a third party may use, but won’t have your source code, don’t just rethrow any random Exception. Throw your own custom Exception if you must, but document that it can do so and what it means, possibly with the random one in the InnerException property.

Of course, if you do throw random Exceptions from a library, it’s probably not going to be used by anyone for long.

Advertisements

About Jerome

I am a senior C# developer in Johannesburg, South Africa. I am also a recovering addict, who spent nearly eight years using methamphetamine. I write on my recovery blog about my lessons learned and sometimes give advice to others who have made similar mistakes, often from my viewpoint as an atheist, and I also write some C# programming articles on my programming blog.
This entry was posted in Programming and tagged . Bookmark the permalink.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s