Spring Boot application code review points

I have more opportunities to touch Spring Boot at work. Spring can make apps with fairly high productivity by making full use of annotations and related functions, but there are many parts that cause unintended behavior when used "somehow". This time, I would like to sort out the content that I often comment on during code reviews (although it is only a rudimentary point ...).

(1) Is it divided into Controller / Service / Repository?

In Spring, it is recommended to divide the processing of the entire application into the following three-layer structure class instead of writing it in a specific class.

Since ** Controller ** is the entry point for requests, I think that developers who are new to Spring will be familiar with it, but if you create an app without thinking about anything, you will end up with ** Controller **. It is easy to implement business logic in **. Then, ** a Fat Controller with low maintainability and readability ** will be created, so I think it is a point to be strongly aware of the isolation of business logic.

Personally, I think it's best to focus on the following contents. The idea is.

• Controller --Reception of requests --Validation of request parameters --Run Service --Exception handling --Creating a response
• Service --Execution of business logic --Cooperation with external services --Data manipulation through Repository --Transaction management
• Repository --Data manipulation

When calling multiple types of Services in a Controller, the Controller tends to be large, so in that case, I feel that it is okay to create a ServiceFacade class that mediates between the Controller and the Service.

(2) Is the implementation considering the life cycle of the instance?

Instances managed by Spring DI container are ** Singleton ** by default. In other words, if you have a state with a member variable etc., the state (user ID etc.) will be mixed among users accessing the same application, and it will often not be the intended behavior. Basically, I think it is better to make it Stateless, but depending on the requirements, please consider changing to another scope supported by Spring.

By the way, if it is your own class, you can change the scope by adding @Scope annotation.

@Scope("prototype")
@Service
public class SecretService {
 //Processing implementation
}

As a check point of view

--Does the Singleton instance have a state? --Are you using another scope (session / prototype) unnecessarily?

The point is around.

(3) Are you using constructor injection?

Dependency injection (DI) is unavoidable when using Spring, but there are the following two methods if you do not write a configuration file. * I think that the definition in XML is still alive, but I think that few people are using it, so I will omit it.

--Field injection --Constructor injection

Field injection is easy because you just add @Autowired to the member variable.

@Autowired
private SecretService service;

However, it has been deprecated due to the following drawbacks. * I remember issuing a warning even in Spring.

--Mocking is not possible without using a DI container when creating test code --The final attribute cannot be added to the field and it cannot be immutable.

So unless you have a very good reason (such as a circular dependency), use constructor injection.

private final SecretService service

public MyClass(SecretService service) {
  this.service = service;
}

If the definition of the constructor is complicated, you can also use Lombok.

@AllArgsConstructor
public class MyClass {
  //TODO:Implementation
}

(4) Is there any processing that can be shared?

Spring provides HandlerInterceptor and AOP as a mechanism to define cross-cutting common processing. By utilizing these mechanisms

--Perform common preprocessing for all methods of all classes under a specific package --Common post-processing with methods that return a specific type of return value

In some cases, you don't have to implement duplicate processing. Is there a lot of ** log output ** as a common case?

  /**
   *Logger class
   */
  private final Logger logger = LoggerFactory.getLogger(MyController.class);
  
   
  /**
   *Request method sample.
   * @param form Form class
   * @param bindingResult Validation result
   * @return response
   */
  @RequestMapping(value = "/", method = RequestMethod.GET)
  public String handleRequest(@Valid MyControllerForm form, BindingResult bindingResult) {
    logger.info("Interceptor test");
    //abridgement
  }
}

Specifically

@Aspect
@Component
public class MyInterceptor {

  @Before("execution(* jp.co.cross_xross.controller..*.*(..))")
  public void beforeProcess(JoinPoint joinPoint) throws Throwable {
    //Preprocessing
  }
}

You can execute "pre-processing", "post-processing", and "alternative processing" by specifying the package name, method name, arguments, and return value as in.

(5) Have you implemented the functions provided by Spring yourself?

The material is running out soon (laughs) Spring provides functions that are often implemented as subprojects. I think that it is better to utilize the part that tends to be difficult to implement by yourself because it can be easily realized by using this function.

Afterword ...

I would like to confirm it together with the basic code review points of Java. From the perspective of pure Java code review, I refer to the articles of seniors written in qiita in the past.

-Reference information for successful method naming -Let's write some nice Java code -Code Review Checklist