Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw exceptions in a consistent manner #116

Closed
rimadoma opened this issue Aug 9, 2018 · 0 comments · Fixed by #130
Closed

Throw exceptions in a consistent manner #116

rimadoma opened this issue Aug 9, 2018 · 0 comments · Fixed by #130

Comments

@rimadoma
Copy link
Contributor

rimadoma commented Aug 9, 2018

The code in the Modern modules of BoneJ should throw exceptions in a consistent manner.

Exceptions related to user-input, I/O and other areas where exceptions are to be expected, should be caught with a catch block. The user should be informed of the error, and the stack trace logged with a call to LogService.trace(). This way the exception info can be viewed by selecting Edit > Options > ImageJ2... and setting SciJava log level to TRACE if need be. Normally the user shouldn't see these kinds of exceptions. See issue #82.

NullPointer, IllegalArgument and other exceptions that result from programmer errors should not be caught. Otherwise these bugs might go unnoticed. These exceptions should only be thrown if they otherwise wouldn't already happen in the scope, or when they add new information about valid behaviour or data. Some examples:

// Example 1
public static char initial(final String name) {
  // Throwing a NullPointerException would be redundant,
  // because the next line will throw it if name == null 
  return name.charAt(0);
}
// Example 2
public class Employee {
  private String name;

  public String getName() { return name; }

  public void setName(final String name) throws NullPointerException {
    // Exception is thrown, because otherwise there's no telling when the null name field
    // would cause trouble
    if (name == null)
      throw new NullPointerException("Name cannot be null!");
    this.name = name;    
  }
}
// Example 3
public void setRadius(final double r) {
  // Exception thrown, because it adds a new constraint for valid data
  if (r <= 0) 
    throw new IllegalArgumentException("Radius must be positive!");
  radius = r;
}
// Example 4
public enum Day {
    SUNDAY, MONDAY, TUESDAY, WEDNESDAY, 
    THURSDAY, FRIDAY, SATURDAY 
}

public String dayName(final Day currentDay) {
  switch(currentDay) {
      case SUNDAY: 
        return "Sunday";
      ...
      case MONDAY:
        return "Monday";
      default:
        // Exception thrown, because code has entered a bad path
        throw new RuntimeException("Somebody forgot to cover all valid cases!");
  }
}

Code that migrates from BoneJ to the framework should follow the exception conventions presented there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant