클린 코드 (함수)

dasd412·2022년 7월 18일
0

1. switch 문

switch문은 본질적으로 한 가지 작업만 처리하기 힘든 구문이다.
그래서 되도록 사용하지 말고, 다형적 객체를 생성하는 코드 안에서만 단 한 번 사용하라고 한다.

내가 작성했었던 코드 예시

인터페이스 코드

GoogleUserInfo,FaceBookUserInfo,GitHubUserInfo 등의 구체 클래스를 인터페이스로 추상화했다.

/**
 * OAuth 요청이 오면, 사용자가 어떤 provider를 통해 로그인했는지 알기 위해 사용되는 인터페이스.
 */
public interface OAuth2UserInfo {
    /**
     * @return Oauth 제공 회사 이름
     */
    String getProvider();

    /**
     * @return OAuth provider 가 제공한 "유일한" 식별자 값
     */
    String getProviderId();//

    String getEmail();

    String getName();
}

팩토리 코드

@Component
public class OAuth2UserInfoFactory {

    public Optional<OAuth2UserInfo> selectOAuth2UserInfo(OAuth2User oAuth2User, String registrationId) {
        switch (registrationId) {
            case "google":
                return Optional.of(new GoogleUserInfo(oAuth2User.getAttributes()));
            case "facebook":
                return Optional.of(new FaceBookUserInfo(oAuth2User.getAttributes()));
            case "github":
                return Optional.of(new GitHubUserInfo(oAuth2User.getAttributes()));
            default:
                return Optional.empty();
        }
    }
}

클라이언트 코드

@Service
public class PrincipalOAuth2UserService extends DefaultOAuth2UserService {
	private final OAuth2UserInfoFactory oAuth2UserInfoFactory;
    
    @Override
    public OAuth2User loadUser(OAuth2UserRequest oAuth2UserRequest) throws OAuth2AuthenticationException {
        OAuth2User oAuth2User = super.loadUser(oAuth2UserRequest);
        /* OAuth 로그인의 경우 OAuth Provider 정보가 필요하다. */
        OAuth2UserInfo oAuth2UserInfo = selectProvider(oAuth2User, oAuth2UserRequest).orElseThrow(() -> new IllegalStateException("등록된 provider 가 아닙니다."));

        String username = oAuth2UserInfo.getProvider() + "_" + oAuth2UserInfo.getProviderId();

        /* OAuth 로그인의 경우 비밀번호는 사용되지 않는다. */
        String password = "diabetesdiaryapi";

        Role role = Role.User;

        /* 만약 회원가입된 적 없으면, writerService 한테 회원 가입을 요청한다. 비밀 번호 인코딩도 해준다. */
        Writer writer = writerRepository.findWriterByName(username)
                .orElseGet(() ->
                        writerService.saveWriterWithSecurity(username,
                                oAuth2UserInfo.getEmail(),
                                password,
                                role,
                                oAuth2UserInfo.getProvider(),
                                oAuth2UserInfo.getProviderId()));

        /* 리턴 객체는 스프링 시큐리티 세션 내의 Authentication 에 담기게 된다. */
        return new PrincipalDetails(writer, oAuth2User.getAttributes());
    }
    
    private Optional<OAuth2UserInfo> selectProvider(OAuth2User oAuth2User, OAuth2UserRequest userRequest) {
        String registrationId = userRequest.getClientRegistration().getRegistrationId();
        return oAuth2UserInfoFactory.selectOAuth2UserInfo(oAuth2User, registrationId);
    }
}

음... 그런데 PrincipalOAuth2UserService 내의 코드
String registrationId = userRequest.getClientRegistration().getRegistrationId();의 경우 OAuth2UserInfoFactory 내의 selectOAuth2UserInfo()안에서 처리하는 게 더 응집력이 높아 보인다. 그렇게 되면 selectProvider()는 단순 리턴만 하므로 쓸모가 없다. 그래서 삭제한다.

리팩토링 결과 (switch문이랑 크게 연관은 없음)

팩토리 코드

@Component
public class OAuth2UserInfoFactory {

    public Optional<OAuth2UserInfo> selectOAuth2UserInfo(OAuth2User oAuth2User, OAuth2UserRequest userRequest) {
        String registrationId = userRequest.getClientRegistration().getRegistrationId();
        switch (registrationId) {
            case "google":
                return Optional.of(new GoogleUserInfo(oAuth2User.getAttributes()));
            case "facebook":
                return Optional.of(new FaceBookUserInfo(oAuth2User.getAttributes()));
            case "github":
                return Optional.of(new GitHubUserInfo(oAuth2User.getAttributes()));
            default:
                return Optional.empty();
        }
    }
}

클라이언트 코드

필요 없게 된 private 메소드 selectProvider()를 삭제하였다.

@Service
public class PrincipalOAuth2UserService extends DefaultOAuth2UserService {

    @Override
    public OAuth2User loadUser(OAuth2UserRequest oAuth2UserRequest) throws OAuth2AuthenticationException {
        OAuth2User oAuth2User = super.loadUser(oAuth2UserRequest);
        /* OAuth 로그인의 경우 OAuth Provider 정보가 필요하다. */
        OAuth2UserInfo oAuth2UserInfo = oAuth2UserInfoFactory.selectOAuth2UserInfo(oAuth2User, oAuth2UserRequest).orElseThrow(() -> new IllegalStateException("등록된 provider 가 아닙니다."));

        String username = oAuth2UserInfo.getProvider() + "_" + oAuth2UserInfo.getProviderId();

        /* OAuth 로그인의 경우 비밀번호는 사용되지 않는다. */
        String password = "diabetesdiaryapi";

        Role role = Role.User;

        /* 만약 회원가입된 적 없으면, writerService 한테 회원 가입을 요청한다. 비밀 번호 인코딩도 해준다. */
        Writer writer = writerRepository.findWriterByName(username)
                .orElseGet(() ->
                        writerService.saveWriterWithSecurity(username,
                                oAuth2UserInfo.getEmail(),
                                password,
                                role,
                                oAuth2UserInfo.getProvider(),
                                oAuth2UserInfo.getProviderId()));

        /* 리턴 객체는 스프링 시큐리티 세션 내의 Authentication 에 담기게 된다. */
        return new PrincipalDetails(writer, oAuth2User.getAttributes());
    }

}


2. 함수 파라미터는 적을수록 좋다.

파라미터가 많을수록 함수를 바로 이해하기 어렵다. 특히 테스트 코드 관점에서는 파라미터 개수의 조합 가짓수만큼 테스트를 작성해야 하므로 끔찍하다. 그리고 같은 타입의 파라미터가 연달아 있으면 실수로 호출하기도 쉽다.

작성 코드 예시

    @Transactional(readOnly = true)
    public List<DiabetesDiary> getDiariesBetweenLocalDateTime(EntityId<Writer, Long> writerEntityId, LocalDateTime startDate, LocalDateTime endDate) {
        logger.info("get Diaries Between LocalDateTime");
        checkNotNull(writerEntityId, "writerId must be provided");
        checkArgument(isStartDateEqualOrBeforeEndDate(startDate, endDate), "startDate must be equal or before than endDate");

        List<Predicate> predicates = new ArrayList<>();
        predicates.add(decideBetweenTimeInDiary(startDate, endDate));
        return diaryRepository.findDiariesWithWhereClause(writerEntityId.getId(), predicates);
    }

위 코드의 문제점은 LocalDateTime startDate, LocalDateTime endDate의 파라미터 부분이다.

어쨋든, 책에 의하면 인수 객체를 생성 후, 해당 객체를 파라미터로 대신 넣으면 개념적 표현으로 우수하다.

추가적으로 좋은 점은, 첫 째 checkArgument() 로직의 책임인수 객체가 갖게 된다.
둘 째, 파라미터 또는 파라미터의 순서가 헷갈릴 확률이 적어진다.

리팩토링 코드

    @Transactional(readOnly = true)
    public List<DiabetesDiary> getDiariesBetweenLocalDateTime(EntityId<Writer, Long> writerEntityId, FromStartDateToEndDate fromStartToEndDate) {
        logger.info("get Diaries Between LocalDateTime");
        checkNotNull(writerEntityId, "writerId must be provided");
     
        List<Predicate> predicates = new ArrayList<>();
        predicates.add(decideBetweenTimeInDiary(fromStartToEndDate));
        return diaryRepository.findDiariesWithWhereClause(writerEntityId.getId(), predicates);
    }

FromStartToEndDate 객체를 생성할 때, 생성자 내에서 checkArgument()를 하면 더 응집력 있는 코드가 될 것 같다.


profile
아키텍쳐 설계와 테스트 코드에 관심이 많음.

0개의 댓글