如何拆分此功能以降低认知复杂性？

Question

我是 Java 新手，这个 noob 功能一直困扰着我。我已经下载了 sonarLint 和 lo，发现它把这个问题抛在了我的脸上方法的认知复杂性不应该太高。我知道它看起来很难看，但谁能指出我如何重新格式化以获得干燥的概念，并且也不像 SonarLint 提到的那样具有高复杂性。

@PostMapping("/adduser")
//  @PreAuthorize("hasRole('ADMIN')")
    public ResponseEntity<MessageResponse> registerUser(@Valid @RequestBody SignupRequest signUpRequest) {
        /*
         * This controller Creates new user based on all the entities for the user
         * 
         */
         if(dbValue)
         {
             if (repository.existsByUsername(signUpRequest.getUsername())) {
                    return ResponseEntity
                            .badRequest()
                            .body(new MessageResponse("Error: Username is already taken!"));
                }

                if (repository.existsByEmail(signUpRequest.getEmail())) {
                    return ResponseEntity
                            .badRequest()
                            .body(new MessageResponse("Error: Email is already in use!"));
                }

                // Create new user's account
                User2 user = new User2(signUpRequest.getUsername(), 
                                     signUpRequest.getEmail(),
                                     signUpRequest.getCustomername(),
                                     signUpRequest.getCustomerid(),
                                     signUpRequest.getDescription(),
                                     encoder.encode(signUpRequest.getPassword()));              
                
                Set<String> strRoles = signUpRequest.getRoles();                
                Set<Role2> roles = new HashSet<>();
                Role2 e = new Role2();
                e.setName("ROLE_ADMIN");
                roles.add(e);               
                user.setRoles(roles);
                repository.save(user);      
                
         }
         
         else {
             
             if (repository.existsByUsername(signUpRequest.getUsername())) {
                    return ResponseEntity
                            .badRequest()
                            .body(new MessageResponse("Error: Username is already taken!"));
                }

                if (repository.existsByEmail(signUpRequest.getEmail())) {
                    return ResponseEntity
                            .badRequest()
                            .body(new MessageResponse("Error: Email is already in use!"));
                }

                // Create new user's account
                User1 user = new User1(signUpRequest.getUsername(), 
                                     signUpRequest.getEmail(),
                                     signUpRequest.getCustomername(),
                                     signUpRequest.getCustomerid(),
                                     signUpRequest.getDescription(),
                                     encoder.encode(signUpRequest.getPassword()));
                

                Set<String> strRoles = signUpRequest.getRoles();
                Set<Role> roles = new HashSet<>();
              
                if (strRoles == null) {
                    Role userRole = roleRepository1.findByName(URole.ROLE_USER)
                            .orElseThrow(() -> new RuntimeException("Error: Role is not found."));
                    roles.add(userRole);
                } else {
                    strRoles.forEach(role -> {
                        switch (role) {
                        case "admin":
                            Role adminRole = roleRepository1.findByName(URole.ROLE_ADMIN)
                                    .orElseThrow(() -> new RuntimeException("Error: Role is not found."));                          
                            roles.add(adminRole);

                            break;
                        
                        default:
                            Role userRole = roleRepository1.findByName(URole.ROLE_USER)
                                    .orElseThrow(() -> new RuntimeException("Error: Role is not found."));
                            roles.add(userRole);
                        }
                    });
                }
                user.setRoles(roles);
                repository.save(user);
         }      
         return ResponseEntity.ok(new MessageResponse("User Added successfully!"));     
    }   

感谢批评和任何帮助。提前致谢。

Answer 1

1. 在控制器中，您需要使用服务，而不是存储库。在那里创建用户创建方法。
2. 用方法创建接口UserValidator（existsByUsername，existsByEmail实现谓词） Optional check(Predicate condition, String message) 或将此方法委托给新的验证实用程序类。
3. 您已复制，可以将其移出条件

if (repository.existsByUsername(signUpRequest.getUsername())) 
    return ResponseEntity
        .badRequest() // below, you always create a new string and a new message response, use a constant as not to clog the memory
        .body(new MessageResponse("Error: Username is already taken!"));
if (repository.existsByEmail(signUpRequest.getEmail())) 
    return ResponseEntity
        .badRequest() // here too
        .body(new MessageResponse("Error: Email is already in use!"));

4. signUpRequest.getRoles() 可能返回一个 Optional，或者 Collections.emptyList() 如果为空而不是 null
5. 通过 .valueOf() 获取角色，它使 .forEach() 条件不被使用，并且也可以使用 switch-case。
6. 替换这个

Role userRole = roleRepository1.findByName(URole.ROLE_USER)
   .orElseThrow(() -> new RuntimeException("Error: Role is not found."));
roles.add(userRole);

与

roleRepository1.findByName(URole.ROLE_USER).ifPresent(roles::add);

Answer 2

只要始终如一地应用Extract Method，它将大大改进您的代码。这样做还可以帮助您发现责任并为进一步重构铺平道路。

您可以按原样使用现有代码，然后将其拆分为更小的自描述方法，以使主要算法更易于理解。

例如

if (repository.existsByUsername(signUpRequest.getUsername())) {
    return usernameTakenError();
}
if (repository.existsByEmail(signUpRequest.getEmail())) {
    return emailUsedError();
}
user = userToSignup(signUpRequest);
userRepository.save(user);
return ResponseEntity.ok(new MessageResponse("User Added successfully!")); 

在这一点上，如何实际实现这些方法并不重要：核心算法仍然简短易懂。

请注意，我经常将异常自动转换为响应错误，这允许以声明性方式检查规则 例如

//throws if taken and a generic exception handler 
//returns and error response
assertUsernameFree(); 

同时寻找 DRY 机会。例如，而不是：

roleRepository1.findByName(role).orElseThrow(() -> new RuntimeException("Error: Role is not found."));

您可以使用 roleRepository1.roleOfName(role) 或 findExistingByName 来抛出而不是返回 Optional。

此外，您的代码的整个角色映射部分太复杂了，我认为其中可能存在错误：default switch case 中添加了任何未知角色为USER_ROLE 似乎并没有对我有意义。

我希望有更多类似的东西（在resolveSignupRoles 函数中，甚至是双重调度signUpRequest.getRoles(rolesRepo)）：

Set<Role> roles = signUpRequest.getRoles().stream()
    .map(r -> URole.fromCode(r))
    .map(r -> userRepository::roleOfName).collect(Collectors.toSet());

if (roles.isEmpty()) roles.add(userRepository.roleOfName(URole.ROLE_USER));

这里有大量的重构机会，但Extract Method 很容易应用，并且会立即改进您编写的所有代码。