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

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

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

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!"));

signUpRequest.getRoles() 可能 returns 一个可选的，或者 Collections.emptyList() 如果一个空的，而不是一个 null
通过 .valueOf() 获取角色，它使 .forEach() 条件未使用并且 switch-case 也是如此。
替换这个

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，这将大大改进您的代码。这样做也将帮助您发现职责并为进一步重构铺平道路。

您可以按原样使用现有代码，将其拆分为更小的 self-descriptive 方法，以使主要算法更易于理解。

例如

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 函数甚至 double-dispatch 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 易于应用并且会立即改进您编写的所有代码。

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

How can I split this function for low congnitive complexity?

java

spring-boot

sonarlint