How to follow Uncle Bob's rule(recommendation) to use one try-catch block in one method correctly?

user_11926338 0
Viewed 162

For example, I have a method

void process(String userId) {
  if(userId == null) throw new IlligalArgumentException("Usesr ID is required);

  User user = userService.findUserById(userId);

  if(user == null) throw new UserNotFoundException("User with ID: "+ userId +" not found");
  
  try {
     DataResponse response = analyticsAPI.loadAnalytics(userId, user.getDob(), user.getFirstName());  

     //logic
   } catch(AnalyticsAPIException e) {
     //logic
   }
 }
  1. IlligalArgumentException is the unchecked exception
  2. UserNotFoundException is the unchecked exception
  3. AnalyticsAPIException is the checked exception

I read that it is a best practice to start the method from try and finish with a catch instead of multiplying try-catch blocks in the one method.

Prefer exceptions to error codes We prefer exceptions to error codes because they are more explicit. When dealing with try / catch, we should not add more logic in a function than the try / catch block, so that function does one thing: handle errors. Recommendation: don’t use nested try / catch.

Something like this:

void process(String userId) {
  try {
      if(userId == null) throw new IlligalArgumentException("Usesr ID is required);
    
      User user = userService.findUserById(userId);
    
      if(user == null) throw new UserNotFoundException("User with ID: "+ userId +" not found");
      
         DataResponse response = analyticsAPI.loadAnalytics(userId, user.getDob(), user.getFirstName());  

         //logic
       } catch(AnalyticsAPIException e) {
         //logic
       }
     }

But it looks strange. I throw an exception inside of the try-catch block and hope that it won't be handled in the catch. I expect that it will be thrown upper to the service which called that method.

I can do next:

public void process(String userId) {
          try {
              if(userId == null) throw new IlligalArgumentException("Usesr ID is required);
            
              User user = userService.findUserById(userId);
            
              if(user == null) throw new UserNotFoundException("User with ID: "+ userId +" not found");
              
              DataResponse response = callApi(userId, user.getDob(), user.getFirstName());  
        
              //logic
             }
    
private DataResponse callApi(String userId, Date dob, String firstName){
               try {
                 return analyticsAPI.loadAnalytics(userId, user.getDob(), user.getFirstName());  
               } catch(AnalyticsAPIException e) {
                 //logic
               }
         }

But it doesn't work always. So, what is the better?

clean-architecture refactoring architecture exception java
1 Answers

What unclebob proposes is that you don't have a list of statements in the try or catch block. Instead you should separate the "normal" case from the exception case. The goal is to separate different levels of abstraction.

To avoid name collisions I often prefix the "normal" case with the "try" word. I also often separate the try/catch in an own method to keep things focused. E.g.

The "process" method should focus on what "to process a user (userId)" means. It is one level of abstraction and if you separate it from the other methods it is easier to read and understand.

void process(String userId) {
    User user = getUserById(userId);

    loadAnalytics(user);
}

The getUserById focuses only on the logic that is required when you want to get a user by it's id.

void void getUserById(String userId){
    if(userId == null) throw new IlligalArgumentException("Usesr ID is required");
    
    User user = userService.findUserById(userId);
    if(user == null) throw new UserNotFoundException("User with ID: "+ userId +" not found");
    
    return user;
}

To understand the "process" method it is not necessarry to understand that loading analytics can cause exceptional states or even how they are handled.

But when you dive into the loadAnalytics method you want to know how it works. Now you can immediately see that loading analytics might cause exceptional states. The method is focused on the exception handling, because it only contains the try/catch and you also focus on the kind of exceptions that can occur.

void loadAnalytics(User user){
    try {
     tryLoadAnalytics(user);
    } catch(AnalyticsAPIException e) {
     handleAnalyticsError(e);
    }
}

I often use the "try" prefix to avoid name collisions and to make clear that a method might fail.

void tryLoadAnalytics(){
    DataResponse response = callApi(userId, user.getDob(), user.getFirstName());  
    
    //logic
}

Like the "normal" case the exception handling is separated so that you can focus on how a specific exception is handled.

void handleAnalyticsError(AnalyticsAPIException e){
    //logic
}
user_9741860
Related