Error Handling
Returning error codes leads to deeply nested structures.
If the keyword try exists in a function, it should be the very first word in the function and there should be nothing after the catch/finally blocks.
public void performLogin(UserDto userDto) {
try {
login(userDto);
} catch(Exception ex) {
log.error(ex.getMessage(), ex);
throw new BadCredentialExceptoin();
}
redirect(); // Should move to try block
}
public void performLogin(UserDto userDto) {
try {
login(userDto);
redirect();
} catch(Exception ex) {
log.error(ex.getMessage(), ex);
throw new BadCredentialExceptoin();
}
}
The checked exception is an Open/Closed Principle violation. If you throw a checked exception from a method in your code and the
catch is three levels above, you must declare that exception in the signature of each method between you and the catch. This means that a change at a low level of the software can force signature changes on many higher levels. The changed modules must be rebuilt and redeployed, even though nothing they care about changed.Encapsulation is broken because all functions in the path of a throw must know about the details of that low-level exception.
Checked exceptions can sometimes be useful if you are writing a critical library: You must catch them. But in general application development, the dependency costs outweigh the benefits.
class UserNotFoundException extends Exception { // Checked exception
public UserNotFoundException(String message) {
super(message);
}
}
class AuthException extends Exception { // Checked exception
public AuthException(String message) {
super(message);
}
}
public class AuthController {
private AuthService authService;
public void login(AuthDto dto) {
try {
authService.auth(dto);
} catch(UserNotFoundException ex) {
} catch(AuthException ex) {
}
}
}
public class AuthService {
private AuthService authService;
// Must rethrow or catch a checked exception
public void login(AuthDto dto) throws UserNotFoundException, AuthException {
User user = getUserByEmail(dto.getEmail());
if(notMatch(user.getPasword(), dto.getPassword()) {
throw new AuthException("User does not exist");
}
// Login success
}
}
public class UserService throws UserNotFoundException {
private UserRepository userRepository;
public User getUserByEmail(String email) throws UserNotFoundException {
User user = userRepository.getUserByEmail(email);
if (user == null) {
throw new UserNotFoundException("User does not exist");
}
return user;
}
}
class UserNotFoundException extends RuntimeException { // Unchecked exception
public UserNotFoundException(String message) {
super(message);
}
}
class AuthException extends RuntimeException { // Unchecked exception
public AuthException(String message) {
super(message);
}
}
public class AuthController {
private AuthService authService;
public void login(AuthDto dto) {
try {
authService.auth(dto);
} catch(UserNotFoundException ex) {
} catch(AuthException ex) {
}
}
}
public class AuthService {
private AuthService authService;
public void login(AuthDto dto) {
User user = getUserByEmail(dto.getEmail());
if(notMatch(user.getPasword(), dto.getPassword()) {
throw new AuthException("User does not exist");
}
// Login success
}
}
public class UserService throws UserNotFoundException {
private UserRepository userRepository;
public User getUserByEmail(String email) {
User user = userRepository.getUserByEmail(email);
if (user == null) {
throw new UserNotFoundException("User does not exist");
}
return user;
}
}
Each exception that you throw should provide enough context to determine the source and location of an error.
class UserNotFoundException extends RuntimeException { // Unchecked exception
public UserNotFoundException(String message) {
super(message);
}
}
class AuthException extends RuntimeException { // Unchecked exception
public AuthException(String message) {
super(message);
}
}
public class AuthController {
private AuthService authService;
public void login(AuthDto dto) {
try {
authService.auth(dto);
} catch(AuthException ex) {
log.error("Auth failed", ex);
}
}
}
public class AuthService {
private AuthService authService;
public void login(AuthDto dto) {
try {
User user = getUserByEmail(dto.getEmail());
if(notMatch(user.getPasword(), dto.getPassword()) {
throw new AuthException("User does not exist");
}
// Login success
} catch (UserNotFoundException ex) {
throw new AuthException("Can not get user"); // Missing stack trace from root exception
}
}
}
public class UserService throws UserNotFoundException {
private UserRepository userRepository;
public User getUserByEmail(String email) {
User user = userRepository.getUserByEmail(email);
if (user == null) {
throw new UserNotFoundException("User does not exist");
}
return user;
}
}
class AuthException extends RuntimeException { // Unchecked exception
public AuthException(String message, Throwable cause) {
super(message, cause); // Accept stack trace from root exception
}
}
public class AuthService {
private AuthService authService;
public void login(AuthDto dto) {
try {
User user = getUserByEmail(dto.getEmail());
if(notMatch(user.getPasword(), dto.getPassword()) {
throw new AuthException("User does not exist");
}
// Login success
} catch (UserNotFoundException ex) {
throw new AuthException("Can not get user", ex); // Provide the cause of exception
}
}
}
We should simplify our code considerably by wrapping the API that we are calling and making sure that it returns a common exception type.
public void run() {
try {
LocalPort port = new LocalPort();
port.open();
} catch (DeviceResponseException e) {
reportPortError(e);
logger.log("Device response exception", e);
} catch (ATM1212UnlockedException e) {
reportPortError(e);
logger.log("Unlock exception", e);
} catch (GMXError e) {
reportPortError(e);
logger.log("Device response exception");
} finally {
// …
}
}
public class LocalPort {
public void open() throws DeviceResponseException, ATM1212UnlockedException, GMXError {
// Do something
}
}
public void run() {
try {
LocalPort port = new LocalPort();
port.open();
} catch (PortDeviceFailure e) {
reportError(e);
logger.log(e.getMessage(), e);
} finally {
// …
}
}
public class PortDeviceFailure extends RuntimeException { // exception wrapper
public PortDeviceFailure(String message, Throwable cause) {
super(message, cause);
}
}
public class LocalPort {
public void open() {
try {
// Do something
} catch (DeviceResponseException e) {
throw new PortDeviceFailure("Device response exception", e);
} catch (ATM1212UnlockedException e) {
throw new PortDeviceFailure("Unlock exception", e);
} catch (GMXError e) {
throw new PortDeviceFailure("Device response exception", e);
}
}
}
Wrapping third-party APIs is a best practice. When you wrap a third-party API, you minimize your dependencies upon it:
- You can choose to move to a different library in the future without much penalty.
- Wrapping also makes it easier to mock out third-party calls when you are testing your own code.
Returning null maybe leads to deeply nested structures.
public void registerItem(Item item) {
if (item != null) {
ItemRegistry registry = peristentStore.getItemRegistry();
if (registry != null) {
Item existing = registry.getItem(item.getID());
if (existing.getBillingPeriod().hasRetailOwner()) {
existing.register(item);
}
}
}
}
It’s easy to say that the problem with the code above is that it is missing a
null check, but in actuality, the problem is that it has too many. If you are tempted to return null from a method, consider throwing an exception or returning a SPECIAL CASE object instead.List<Employee> employees = getEmployees();
if (employees != null) {
for (Employee e : employees) {
totalPay += e.getPay();
}
}
public List<Employee> getEmployees() {
return null;
}
List<Employee> employees = getEmployees();
for (Employee e : employees) {
totalPay += e.getPay();
}
public List<Employee> getEmployees() {
return Collections.emptyList();
}
Consider returning an
empty list instead of a null valueReturning null from methods is bad, but passing null into methods is worse. Unless you are working with an API which expects you to pass null, you should avoid passing null in your code whenever possible.
public double xProjection(Point p1, Point p2) {
return (p2.x – p1.x) * 1.5;
}
We’ll get a NullPointerException if a caller passes a null parameter.
How can we fix it? We could create a new exception type and throw it:
public double xProjection(Point p1, Point p2) {
if (p1 == null || p2 == null) {
throw InvalidArgumentException("Invalid argument for MetricsCalculator.xProjection");
}
return (p2.x –p1.x) *1.5;
}
// alternative
public double xProjection(Point p1, Point p2) {
assert p1 != null : "p1 should not be null"; // throws AssertionException
assert p2 != null : "p2 should not be null"; // throws AssertionException
return (p2.x –p1.x) *1.5;
}
It’s good documentation, but it doesn’t solve the problem. If someone passes
null, we’ll still have a runtime error.In most programming languages there is no good way to deal with a null that is passed by a caller accidentally. Because this is the case, the rational approach is to forbid passing null by default. When you do, you can code with the knowledge that a null in an argument list is an indication of a problem, and end up with far fewer careless mistakes.