MET04-J. Always provide feedback about the resulting value of a method

According to [[Ware 08]], methods should always return a value that allows the developer to know the current state of the object and/or the result of the operation. This is consistent with the advice in [jg:EXP02\-J. Do not ignore values returned by methods]. The returned value should be as representative of the last known state as possible and should be designed keeping in mind the perceptions and mental model of the developer.

Feedback can also be provided by throwing exception objects derived from the Exception class. With this approach, the developer can still get precise information about the outcome of the method and proceed to take the necessary actions. To do so, the exception thrown should provide a detailed account of the abnormal condition at the appropriate abstraction level.

To gain better ability at telling apart correct from fallacious results and enforcing that the incorrect results be carefully handled, a combination of the aforementioned approaches is recommended. At the same time, there are cases when an error value should be returned instead of an exception and vice versa. For instance, if some method is capable of failing in a variety of different ways, it is better to return failure codes than endeavoring to throw scores of different exceptions. Not that no possible failure codes should be within the range of valid return values.

Sometimes a state testing method [[Bloch 08]] can be used to ensure that the object is in consistent state at all points in time. This approach is not useful in the absence of external synchronization. Also, there is a TOCTOU race condition between invocation of the object's state testing method and the call to a method that depends on its state. During this interval, the object's state could change surreptitiously.

Noncompliant code example

As shown in this noncompliant example, methods that are susceptible to failure can silently corrupt the state of the object if they do not return a value that the developer can interpret.

public void updateNode(int id, int newValue){		
  Node current = root;
  while(current != null){
    if(current.getId() == id){
      current.setValue(newValue);
      break;
    }
    current = current.next;
  }
}

Compliant solution

A recommended solution is to return the result of the operation; true for success and false for failure.

public boolean updateNode(int id, int newValue){		
  Node current = root;
  while(current != null){
    if(current.getId() == id){
      current.setValue(newValue);
      return true;
    }
    current = current.next;
  }
  return false;
}

Compliant solution

Another informative solution returns the updated Node so that the developer can simply check for a null value lest the operation fails. Appropriate return values for methods can vary depending on the control flow or the information that the developer finds more useful.

public Node updateNode(int id, int newValue){	
  Node current = root;
  while(current != null){
    if(current.getId() == id){
      current.setValue(newValue);
      return current;
    }
    current = current.next;
  }
  return null;
}

Compliant solution

This solution combines the best of both worlds - exceptions and status codes. In this case, an exception is thrown if the operation is not successful. This ensures that the client has to handle the event wherein the Node is not found.

public Node updateNode(int id, int newValue) throws IdNotFoundException {
  Node current = root;
  while(current != null){
    if(current.getId() == id){
      current.setValue(newValue);
      return current;
    }
    current = current.next;
  }	
  throw new NodeNotFoundException();
}

Risk assesment

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

References

[[Ware 08]]

MET03-J. For methods that return an array or collection prefer returning an empty array or collection over a null value      10. Methods (MET)      MET05-J. Validate method parameters