What is a good practise of executing some operation and returning boolean value indicating successfulness of that in Java?

Question

What I mean is, I read that a good way to write a method is by being guided with a rule: one method should do only one task. And if I have different sequential operations, then I need to split the method into several ones. It should make code cleaner and simple, self-explaining method names. But if I want to implement method which should do something and then return boolean value – true is success, false if failed. For example, assume we have setter called setObjectValue(). [again, it is just an example].

Question: Would it be good to use this name and return boolean values, or should it be something as: isSuccessfullsetObjectValue(), setObjectValueAndCheckIsOk(), or should there be two methods or what? Because name "setObjectValue()" doesn't tell you that the method is doing something besides setting value.

Answer 1

Unless there's a good reason, I would normally use Exceptions to indicate this. This has two benefits:

1. You follow the convention of 1 method - 1 idea
2. You force yourself (if the Exception is checked) to handle the failure case. If you return a boolean, then the code could easily ignore this case.

If you do something like this:

try{

  setObjectValue("foo")
} catch(SomeKindOfException e){
  //handle
} 

Then you get the further benefit of it reading like English: "try to set the object value, but if you can't then handle it by ... "

Answer 2

It really depends on what your code is doing, but from what you described it makes perfect sense and is probably desirable behavior (again depending on what your code is doing).

For example, Collection returns a boolean value if it was changed as a result of the operation. Another example of this is AtomicBoolean. In these cases, since you may need to know if something was modified it makes sense to return a boolean. The naming of a method really doesnt matter as long as it makes sense.

In these examples, it is very relevant to know if whether your set was successful and this is the only place to realistically do this. However, I would not have a method used to set some value which also does a lot of other non-related manipulations.

Additionally, if the reason you would return false in your example is the result of some validation error, you most likely want to check that before attempting to set the value.

Answer 3

In my opinion the answer of James is pretty good. But think about some more setters and the resulting try-catch-blocks. A slightly different approach is to handle your values for these setters through a validator, e.g. the user has made some input or something like this.

String userInput = ...;
if (myValidator.isValid(userInput)) {
   myObject.setObjectValue(userInput);
}

The method isValid(boolean valueToCheck) is indicating, that it will return a boolean value. Your setObjectValue(String newValue) just have to do the job without returning any value.

You can still check the newValue in your setter. If this is an invalid input, you would throw an IllegalArgumentException (fail-fast).

if (newValue==null || newValue.contains("foo")) {
  throw new IllegalArgumentException("Illegal value for newValue: "+newValue);
}
this.value = newValue;

So in your code, you can use your setters for your own values. I mean, you wrote this method and should know your inputs, so this should be fine at all. If the user has made some inputs, use the validator for your setter. There will be strange inputs, believe me! ;-)

The result is, you don't have to handle this amount of try-catch blocks and know where to look if you got an exception (if you don't use the validator).

Answer 4

From my point of view, a setter should not return a value, unless you make a call to slow speed storage device (like a Web Service), but, throwing an exception on such cases will be cleaner.

Answer 5

I think returning a boolean value without explicitly saying what the value represents in the method signature is okay provided you mention what the return value is to be used for in the JavaDoc. This is done throughout the Java Collections API (not that the Collections API is the gold standard), so it's generally an accepted practice in Java to do this.

Answer 6

Setters generally should be void (not return a value). There are several ways to accomplish your goal without making your setter return a boolean. One way would be to set a boolean flag as an instance variable that gets set to true whenever you set a particular value. This flag could be accessed via a getter of its own.

For example, you've got a class Person with Name and Phone Number attributes. You want to be able to set a person's phone number and later determine whether or not this phone number has been set:

public class Person 
{
   private String name;
   private String phone;
   private boolean hasPhone;

   public void setName(String name)
   {
      this.name = name;
   }

   public String getName()
   {
      return name;
   }

   public void setPhone(String phone)
   {
      this.phone = phone;
      hasPhone = true;
   }

   public String getPhone()
   {
      return phone;
   }

   public boolean hasPhone()
   {
      return hasPhone;
   } 
}

Answer 7

It is OK to create a method that checks some conditions and returns a boolean, and another method that might use condition checking before performing some operations.

boolean checkSomeCondition();

And your code will not suffer

if (checkSomeCondition()) {...}
else {...}

But in cases when the method could not perform actions stated in its name, exceptions should be used. They were actually created for that.

try {
   DoSmth();
} catch (ParticularError e) {
   ...
}

It is generally used style, and probably the best way to handle errors that ocurred inside the body of the function.