Question

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.

Was it helpful?

Solution

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.

OTHER TIPS

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.

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 )

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.

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.

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.

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.

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.

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.

Licensed under: CC-BY-SA with attribution
scroll top