Do Not Return True/False on a Transaction Success/Fail

Technorati Tags: C#

There are many times in some shops I’ve worked where I’ve seen developers return a boolean back form a method for a transaction fail vs. succeed as a way to determine whether to do something next in code. A transaction could be any type of work. I talk about transaction here in a generic way; insert into a db, copying of a file to a directory, you name it.

Here’s an example:

public bool DoSomething(int someID, ..., ...)

    return db.DoSomething(someID, ..., ...);

In this example the caller would get a true/false returned based on if the code was successful or not.

Why is this horribly a bad way to handle success/failure of something that “happened”? Well here are 2 obvious reasons:

1) The caller gets a false back. Ok, what the hell does that mean? What if I need to know why for obvious debugging/troubleshooting purposes, especially if I’m gonna call this method from a service

2) I’m not returning something back that is useful. A descriptive error is useful. If not that then I should be sending back a more useful object that I can do something with. No, a boolean in this case is not a smart idea. It tells me something happened but nothing more. It keeps me blind especially if I ever have to debug further at any time to figure out why it was false!

To me, that’s a very lazy coding unless you want to purposely eat the error. In this circumstance there’s no reason to eat it basically by sending back a false. If you’re logging it fine but again, to the caller it’s not helping them at all. While it might work, it’s not going to help when you keep getting false back and wished that the method would be much more helpful and not so mysterious.

And you really shouldn’t be eating the error unless you have a good reason for it. In this case you’re not handling anything and you’re not doing your caller any good or the rest of the code to be built on top of this because then that code becomes lazy also because all it has to do is check for false. Soon all your code is checking for true/false and you have no real debugging capabilities when you wonder why am I getting false, false, false when you step through your code on a bunch of method calls to try to figure out wtf is going on.

This is nothing new, just a reminder again to write write clean maintainable code.

Print | posted on Friday, September 18, 2009 9:45 PM

Comments on this post

# re: Do Not Return True/False on a Transaction Success/Fail

So what would you suggest? Custom Exceptions?

Left by Tayo on Sep 19, 2009 9:48 AM

# re: Do Not Return True/False on a Transaction Success/Fail

what is a suggested alternative?

Left by brian on Sep 19, 2009 11:49 AM

# re: Do Not Return True/False on a Transaction Success/Fail

I'm talking strictly about operations outside of checking for status/state. So for example these would be valid to return a true/false:

hasExcecutePermissions (this could be a property or method)
userIsNew

stuff like this. Where you're checking state where a true/false makes more sense because the intent is to check status on something.

But when you're performing a transaction such as Updating/Inserting/Deleting records, DoSomeProcessing (such as a credit card transaction, an order, or whatever), you should be returning meaningful messages or objects...something very specific. A bool is too general and has no value. You are not required to use the objects returned. So if you don't want to, you don't have to. But at least return something such as the order, the updated record, the inserted record or whatever.

1) Return a useful error message. Then you can catch it and handle it. With a true/false you only know that it failed, but you do not know the best way to always handle it if you don't have a meaningful message bubbled up from the exception class. If I just get a false back, how the hell do I know exactly what happened? How do I know how to properly handle it and if I should ignore it nor not? How do I know anything other than false? Bad.

2) Return a useful object, not a bool. For example return another type, then the caller can do proper handling and check for null. If it's not null then don't use the returned object.

simple as that. Returning a true/false is lazy coding. You can't handle it properly and you're blind to what happened and basically your entire application is going to be based on a true/false scenario instead of actually handling exceptions or checking for null reference or empty strings which is what you should be doing in OOP.

Now there are some exceptions such as hasAccess where you want to check whether someone has rights to perform an action. Otherwise, do not be lazy with your returns.

Returning a bool does nobody any good for those operations other than checking for permissions. What if another developer on your team wonders what the heck false is going to do for me? Good developers will look for something returned that you can do something with but do it with confidence. Checking for a false is not a confident check, it's a blind check...a hack.

Left by Dave Schinkel on Sep 19, 2009 2:02 PM

# re: Do Not Return True/False on a Transaction Success/Fail

For these sorts of things I strongly agree, never return a bool. I always have a base Message class which can be explicitly cast to bool (it just forwards its bool Successful {get;} property when cast) and include additional metadata about the transaction when appropriate. Even on a success this metadata can be important!

Left by Charles Feduke on Sep 24, 2009 11:22 AM