36

If there is a method

bool DoStuff() {
    try {
        // doing stuff...
        return true;
    }
    catch (SomeSpecificException ex) {
        return false;
    }
}

should it rather be called IsStuffDone()?

Both names could be misinterpreted by the user: If the name is DoStuff() why does it return a boolean? If the name is IsStuffDone() it is not clear whether the method performs a task or only checks its result.

Is there a convention for this case? Or an alternative approach, as this one is considered flawed? For example in languages that have output parameters, like C#, a boolean status variable could be passed to the method as one and the method's return type would be void.

EDIT: In my particular problem exception handling cannot be directly delegated to the caller, because the method is a part of an interface implementation. Therefore, the caller can't be charged with dealing with all the exceptions of different implementations. It is not familiar with those exceptions. However, the caller can deal with a custom exception like StuffHasNotBeenDoneForSomeReasonException as was suggested in npinti's answer and comment.

10
  • 2
    It depends on the usage of the function and the stuff that is being done. If it is necessary, that the stuff needs to be executed correctly, then I would consider this approach flawed, because the user of the function might miss the boolean flag and also lacks information provided by the exception.
    – Benni
    Jun 11, 2014 at 9:49
  • 7
    Most of the time, I'd call it "broken" as returning a boolean instead of wrapping or passing the exception is nearly always wrong.
    – maaartinus
    Jun 11, 2014 at 14:54
  • 2
    This kind of exception handler is rarely a good idea. Only catch specific exceptions you're expecting, not all of them. And if possible avoid throwing it in the first place, eliminating the need to catch it. Jun 11, 2014 at 17:12
  • 2
    Umm...BadlyDesignedMethodInSeriousNeedOfRefactoring? And to answer your question about the exceptions - I'd either let the caller handle them, or catch them and then throw a custom exception that means "this method no do its job". Share and enjoy. Jun 11, 2014 at 22:54
  • 5
    To all those who are saying: just throw (or let pass) an exception, you are making unfounded assumptions on how this code is being used. One possible scenario is that there is a problem to be solved, with various heuristic solution methods that solve ever bigger subclasses of the problem at ever increasing cost; it would make sense to write something like if (FirstMethodSucceeds(problem) or SecondMethodSucceeds(problem) or ...) Hurray(); else UniversalSolve(problem);. Doing the same with (custom?) exceptions would be uselessly more complicated. Jun 12, 2014 at 12:01

9 Answers 9

69

In .NET, you often have pairs of methods where one of them might throw an exception (DoStuff), and the other returns a Boolean status and, on successful execution, the actual result via an out parameter (TryDoStuff).

(Microsoft calls this the "Try-Parse Pattern", since perhaps the most prominent example for it are the TryParse methods of various primitive types.)

If the Try prefix is uncommon in your language, then you probably shouldn't use it.

3
  • 3
    +1 This is how I have seen this sort of thing done elsewhere. if (TryDoStuff()) print("Did stuff"); else print("Could not do stuff"); is a pretty standard and intuitive idiom in my opinion. Jun 11, 2014 at 14:56
  • 12
    It should be noted that TryDoStuff methods are assumed to fail cleanly and to have no side effect when they return false.
    – Trillian
    Jun 11, 2014 at 22:24
  • FYI, there is also Remove methods for example in the .NET framework that will remove an element from a data structure (e.g. Dictionary), and return a bool. The consumer of the API can decide to consume it or ignore it. However, errors are reported as exceptions.
    – Omer Iqbal
    Jun 25, 2014 at 4:23
18

What if you simply threw the exception to the calling code?

This way you are delegating the exception handling to who ever is using your code. What if, in the future you would like to do the following:

  • If no exceptions are thrown, take action A
  • If (for instance) a FileNotFoundException is thrown, take action B
  • If any other exception is thrown, take action C

If you throw back your exception, the above change would simply entail the addition of an extra catch block. If you leave it as is, you would need to change the method and the place from which the method is called, which, depending on the complexity of the project, can be in multiple locations.

10
  • 1
    What if the exception is of kind that can't be delegated and should be dealt with internally? Jun 11, 2014 at 9:52
  • 2
    @LimboExile: I think that you could still deal with it internally and then, maybe throw another exception, maybe your own. This would illustrate that something that shouldn't have happened took place, but at the same time, you did not expose what is really going under the hood, which I assume is why you want to deal with the exception internally.
    – npinti
    Jun 11, 2014 at 9:57
  • 6
    It's perhaps worth bearing in mind that throwing an exception should be for an exceptional case. If it is common or normal for DoStuff to fail, throwing an exception for the failure case would be akin to controlling program flow with exceptions which is bad. Exceptions also have an inherent performance cost. If DoStuff failing is an uncommon case due to an error, then exceptions definitely are the way to go as @npinti suggests. Jun 11, 2014 at 14:54
  • 1
    @KarlNicoll Whether something is "exceptional" is completely subjective. The main criteria should be whether you want the program to crash if no one does something about the error, and whether you want the possibility of error to be present in the function's type. Additionally, it's not a fact that exceptions are expensive; it depends on the language and how you throw them. Exceptions are cheap in Python, and if you strip the stack trace information they can be cheap in Java as well. Even if there was a performance cost, it's premature optimization to worry about it until you've profiled.
    – Doval
    Jun 11, 2014 at 15:05
  • 1
    @Doval - I agree that it's subjective, which is why OP shouldn't discount npinti's answer entirely, as it could well be the best course of action. My point was that throwing exceptions isn't always the best path to take. There are plenty of cases where a failure doesn't justify throwing an exception and potentially crashing an application. For example, if OP's DoStuff() method cleans up after the inner code throws an exception, and the Post-conditions of the method are still correct, a return code may be a better option as the error has been handled. Jun 11, 2014 at 15:52
8

DoStuff() is enough, and the returning value of the function should be documented, and you don't need to mention it in the function name, looking for many of API available:

PHP

// this method update and return the number affected rows 
// called update instead of updateAndGetAffectedCount
$affected = $query->update(/* values */);

C-Sharp

// Remove item from the List
// returns true if item is successfully removed; otherwise, false.
public bool Remove( T item )
8

In Java, the Collection API defines an add method that returns a boolean. It basically returns whether the add changed the collection. So for a List it will usually return true, since the item was added, while for a Set it might return false, if the item was already there, since Sets allow each unique item at most once.

That said, if you add an item that is not allowed by the collection, for example, a null value, the add will throw a NullPointerException rather than returning false.

When you apply the same logic to your case, why do you need to return a boolean? If it is to hide an exception, don't. Just throw the exception. That way you can see what went wrong. If you do need a boolean, because it might not have been done, for some other reason than an exception, name the method DoStuff(), since that represents the behavior. It does stuff.

1

Might fail for different reasons, Exception, normal conditions, so would use this:

boolean doStuff() - returns true when successful

Important is to document what the boolean means.

1

I usually have methods return an OperationResult (or OperationResult<T>) and set the IsSuccess property on the OperationResult appropriately. If the 'usual' method is void then return OperationResult, if the 'usual' method returns an object then return OperationResult<T> and set the Item property on the OperationResult appropriately. I'd also suggest having methods on OperationResult such as .Failed(string reason) and .Succeeded(T item). OperationResult quickly becomes a familiar type in your systems and developers get to know how to handle it.

0

It really depends on the language that you're using. Like for some languages there are conventions and protocols that really don't exist in many languages or any at all.

For example, in the Objective-C language and the iOS/Mac OS X SDK method names usually go along the lines of:

- (returndatatype) viewDidLoad {
- (returndatatype) isVisible {
- (returndatatype) appendMessage:datatype variablename with:datatype variablename {

As you can see, there's a method there called viewDidLoad. I prefer to use this type of naming whenever I make my own applications. This could be used to check if something was supposed to happen as you said. I believe that you're thinking too deeply about what it is you name your methods. Most methods return the status of what it is they do regardless, take example:

In PHP, mysql_connect() will return false if the connection fails. It doesn't say it will, but it does and the documentation says it does so it can't be misinterpreted to the programmer.

1
  • Ah, but viewDidLoad is more of a callback notification. Users don't call it to load the view. Rather, it is called after the view is loaded. Likewise: isVisible doesn't set the visibility, rather it just returns what the current value. It doesn't do anything.
    – lilbyrdie
    Jun 12, 2014 at 14:14
0

I would like to suggest to use the 'Try' prefix. This is a well known pattern for situations that the method may succeed or not. This naming pattern has been used by Microsoft in .NET Framework (for example, TryParseInt).

But, maybe this is not about naming. It is about how you design your method's input/output parameters.

My idea is that if a method has a return value, then the purpose of calling that method should be to get that return value. So, you should avoid using return type to indicate the status of an operation.

In C#, I prefer to use an out parameter. Using an out I am marking the parameter as a side parameter. Specially that C# 6 will support inline variable declaration.

DoStuff(out var isStuffDone); // The out parameter name clearly states its purpose.
DoStuff(out var _); // I use _ for naming parameters that I am ignoring.
0

I would pick a name based on the primary role of the method. The fact, that method returns boolean value doesn't mandate 'Is' prefix. Let's have some Java code, which uses 'Is' prefix quite extensivelly. Take look at java.io.File.delete(). It returns boolean, but it isn't called isFileDeleted(). The reason is that the primary role of the metod is to delete a file. Someone calls this method with the intention to delete a file.

The boolean is there just to comunicate back the result of the work. On the other hand let's see java.util.Map.isEmpty(). Obviously, you call such a method to find out if the collection is empty. You do not want any stuff done. You just want to find out what the current state is.

So personally, I would definitelly stick with DoStuff().

This is somewhat a very important issue for me. Many times when I maintain legacy code, I do stumble upon something I call personally "lying method". Name suggests action A, but the body does action B. The most bizzare example beeing a getter method changing data in the database.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.