본문 바로가기

TIL(Today I Learned)

TIL-230925('행동대장' 코드리팩토링 - 회원관리 페이지)

📝오늘 공부한 것

  • 실전프로젝트 - '행동대장' 코드리팩토링(로그인/회원가입 페이지)
  • 자바의 정석 공부 - Chapter 10

 

📌코드리팩토링 - 로그인/회원가입 페이지

⛔ 문제점 : 

마이페이지와 마찬가지로

  • 어떤 요청은 유효성 검사 실패에 대한 메시지를 반환하고, 어떤 요청은 반환하지 않은 응답의 일관성 문제
  • class형식으로 작성된 dto
  • 예외처리 시 ClientErrorCode의 중복 코드 발생 문제
  • Transactional의 불필요한 적용 문제

등이 있었다.

 

💯 해결

  • 일관되게 유효성 검사 실패 시 모두 메시지를 반환시키기
  • 데이터의 무결성 유지를 위해 class대신 record로 dto변경하기
  • static import를 통해 ClientErrorCode코드 중복 제거하기
  • Transactional 맞게 걸어주기

를 통해 문제들을 해결하였다.


⛔ 문제점 : 명확하지 않은 메서드명

위쪽의 checkNickname은 닉네임이 사용가능한지 확인하는 메서드이고, 아래쪽의 checkNickname은 중복되었는지 아닌지를 확인하는 메서드이다. 이러한 메서드명은 메서드의 역할을 명확히 표현하지 않아, 코드를 읽고 이해하기 어렵다고 느낄 수 있다.

 

[SignUpService]

 

💯 해결

아래쪽의 중복되었는지 아닌지를 확인하는 메서드인 checkNickname을 직관적인 이름인 existingNickname으로 변경하였다. 명확하고 직관접인 메서드명을 사용하여 코드를 더 이해하기 쉽도록 하고 유지보수성을 높일 수 있었다.

 

[SignUpService]


⛔ 문제점 : 메서드들의 위치

코드 작성 시 중복되는 코드들은 따로 메서드로 만들어서 사용하였다. 그렇게 따로 뺀 메서드들은 모두 클래스의 맨마지막에 위치시켰다. 그러다 보니 코드를 읽다가 그 사용된 메서드가 나오면 스크롤은 맨 아래로 내렸다가 다시 읽던 위치로 가서 코드를 마저 읽어야 하다 보니 가독성이 떨어졌다.

 

💯 해결

중복코드를 따로 뺀 메서드를 사용하는 위치에 가까운 곳에 위치시켜 가독성을 향상시켰다.

    public CommonResponse signup(SignupRequestDto requestDto) {
        String nickname = requestDto.getNickname();
        String email = requestDto.getEmail();
        String password = passwordEncoder.encode(requestDto.getPassword());

        if(existingNickname(nickname)){
            throw new CommonException(DUPLICATE_NICKNAME);
        }

        long emailId = checkEmailSuccessKey(requestDto.getEmail(), requestDto.getSuccessKey());
        emailRepository.deleteById(emailId);

        // 사용자 ROLE 확인
        UserRoleEnum role = UserRoleEnum.USER;
        if (requestDto.isAdmin()) {
            if (!ADMIN_TOKEN.equals(requestDto.getAdminToken())) {
                throw new CommonException(INVALID_ADMIN_TOKEN);
            }
            role = UserRoleEnum.ADMIN;
        }

        User user = new User(nickname, password, email, role);
        User savedUser =  userRepository.save(user);
        if(savedUser == null){
            throw new CommonException(SIGNUP_FAILED);
        }
        return new CommonResponse(SIGNUP_SUCCESS);
    }


    public CommonResponse checkNickname(CheckNicknameRequestDto requestDto) {
        String nickname = requestDto.nickname();
        if (existingNickname(nickname)) {
            throw new CommonException(DUPLICATE_NICKNAME);
        }
        return new CommonResponse(AVAILABLE_NICKNAME);
    }


    private boolean existingNickname(String nickname){
        Optional<User> existingNickname = userRepository.findByNickname(nickname);
        if(existingNickname.isPresent()){
            return true;
        } else {
            return false;
        }
    }


    public CommonResponse sendEmail(SendEmailRequestDto requestDto) {

        userRepository.findByEmail(requestDto.email()).ifPresent(existingUser -> {
            throw new CommonException(DUPLICATE_EMAIL);
        });

        Optional<Email> email = emailRepository.findByEmail(requestDto.email());

        CommonResponse response = new CommonResponse(SEND_EMAIL_CODE);

        String successKey = emailUtil.makeRandomNumber();

        try{
            emailUtil.sendEmail(requestDto.email(), successKey);
        } catch (CommonException e) {
            throw new CommonException(EMAIL_SENDING_FAILED);
        }
        if (email.isEmpty()) {
            emailRepository.save(Email.builder()
                    .email(requestDto.email())
                    .successKey(successKey)
                    .build());
            return response;
        }
        email.get().changeSuccessKey(successKey);
        return response;
    }


    public CommonResponse checkEmail(CheckEmailRequestDto requestDto) {
        checkEmailSuccessKey(requestDto.email(), requestDto.successKey());
        return new CommonResponse(EMAIL_AUTHENTICATE_SUCCESS);
    }

    private long checkEmailSuccessKey(String requestEmail, String successKey) {
        Email email = emailRepository.findByEmail(requestEmail).orElseThrow(
                ()-> new CommonException(NO_ACCOUNT));
        if(!email.getSuccessKey().equals(successKey)){
            throw new CommonException(EMAIL_AUTHENTICATION_FAILED);
        }
        return email.getId();
    }
}

개선할 점💪🏻

처음에는 기능만 잘 작동되면 되는 줄 알았는데, 코드를 리팩토링하면서 좋은 코드, 가독성이 좋은 코드로 만들다 보니 많은 것을 신경 써야 한다는 것을 점점 느끼고 있다. 나만 알아볼 수 있는 코드가 아니라 누가 봐도 쉽게 이해할 수 있을 만큼의 코드가 되도록 노력해야겠다.