[회고] 우아한 테크코스 6기 백엔드 최종 코딩테스트 회고 & 리팩토링 - 온콜

2023.12.17 - [Other/기록] - [회고] 우아한 테크코스 6기 최종 코딩테스트 후기 feat. 아쉬움 가득 ...

[회고] 우아한 테크코스 6기 최종 코딩테스트 후기 feat. 아쉬움 가득 ...

 

 

지난 후기에서 우아한 테크코스 최종 코딩테스트 시험을 치루면서 준비한 것들과 오프라인 시험을 치고 온 후기를 남겼다.

이후로 오픈 카카오톡 채팅방에서 함께 최종 코딩테스트 코드를 리뷰할 사람들을 모집하였고, 시험을 보았던 리포지토리를 공개로 전환하여 PR 생성 + 리드미 작성 후 코드 리뷰를 나눴다.

 

부족한 나의 결과물을 다시 뜯어보는 것은 너무 마음이 아팠으나 😥😥 어렵게 얻은 코딩테스트라는 기회를 5시간 동안 공부한 것으로 만족하고 끝낼 수 없었다. 이번 글에서 코드 리뷰를 기반으로 내가 부족했던 부분을 다시 되새기고, 리팩토링하는 과정을 작성하여 우아한테크코스의 입과를 위한 공부들의 유종의 미를 거두려고 한다.

---

💪 프로젝트 개요

평일, 휴일 순번에 따라 비상 근무일을 배정한다.
이때 비상 근무자는 연속 2일을 근무할 수 없고, 다음 근무자와 순서를 바꿔야 하는 경우에는 앞의 날짜부터 순서를 변경해야 한다.

📝 구현 기능 목록

월과 시작 요일을 입력하는 기능

• 비상 근무를 배정할 월과 시작 요일을 입력하세요>를 출력한다.
• 월과 시작 요일을 입력받는다.
  • 빈 문자열이 아님을 검증한다.
  • 월은 1 ~ 12 사이의 숫자임을 검증한다.
  • 요일은 월,화,수,목,금,토,일 중 하나임을 검증한다.

평일 및 휴일 비상 근무 순번을 입력하는 기능

• 평일 비상 근무 순번대로 사원 닉네임을 입력하세요>를 출력한다.
• 평일 비상 근무 순번을 입력한다.
• 휴일 비상 근무 순번대로 사원 닉네임을 입력하세요>를 출력한다.
• 휴일 비상 근무 순번을 입력한다.
• 사원 닉네임 입력에 대한 검증을 처리한다.
  • 빈 문자열이 아님을 검증한다.
  • 올바른 구분자로 입력되었음을 검증한다.
  • 닉네임은 1자 이상, 5자 이하임을 검증한다.
  • 총 근무자는 최소 5명, 최대 35명임을 검증한다. (평일, 휴일 전체 기준)
  • 평일 혹은 휴일 순번 하나에 각각 1회의 근무자가 편성되었음을 검증한다.

비상 근무일 배정 규칙에 따라 비상 근무자를 배정하는 기능

• 입력된 월과 시작 요일을 기준으로 비상 근무자를 배정한다.
  • 현재 요일을 기준으로 평일 혹은 휴일 순번을 돌면서 배정한다.
  • 만약, 앞서 배정된 비상 근무자가 연속 2일 근무하는 경우, 현재 요일에 해당되는 순번의 다음 근무자가 근무한다.
  • 순번이 끝까지 돌았을 경우, 다시 순번의 처음으로 돌아가도록 구현한다.

배정된 비상 근무자를 출력하는 기능

• 월, 일, 요일과 비상 근무자의 이름을 출력한다.
• 평일이면서 법정 공휴일인 경우에는 (휴일)을 함께 출력한다.

🛠 시스템 흐름도

💎 핵심 로직

근무자를 배정하는 핵심 로직

AssignManager에서 아래와 같이 구현하였습니다.

AssignManager에서 근무자를 배정하는 상세 로직

1. 주어진 순번이 아래와 같은 경우
   
   
2. 기본적으로 근무자를 배정하는 로직입니다. 순번의 제일 앞에 위치한 근무자를 꺼내어 배정표에 추가합니다. 그리고 해당 근무자는 순번의 제일 뒤로 이동합니다.

    private void addFirstWorker(Workers result, Workers workers) {
        Worker worker = workers.popFront();
        result.addBack(worker);
        workers.addBack(worker);
    }

3. 순번의 제일 앞에 위치한 근무자가 배정표의 마지막 근무자와 겹치는 경우입니다.
   

• 제일 앞에 위치한 근무자를 우선 제거하여 별도로 저장해둡니다.

• 그 다음 근무자를 꺼내어 배정표에 추가하고, 해당 근무자를 제일 뒤로 이동시킵니다.

• 저장해둔 근무자를 다시 제일 앞의 순번에 추가합니다.

아래는 3번의 로직을 구현한 코드입니다.

        String holding=turn.popFront();
        String worker=turn.popFront();
        result.addBack(worker);
        turn.addBack(worker);
        turn.addFront(holding);

---

🙇‍♀️ 피드백 반영하기

java.time.Month를 활용하자!

 

요구사항에 모든 년도는 2월 28일까지인 것으로 구현하고, 잘못 입력 시 예외를 반환하라. 라는 내용의 Month와 관련된 요구사항이 있었다. java.time.Month가 있는 것은 알고 있었지만 28일이 고정인 점, 특정 예외 클래스를 반환하고 숫자를 출력해야하는 점이 있어서 바로 직접 구현하였었다. 

public enum Month {
    JANUARY(1, 31),
    FEBRUARY(2, 28),
    MARCH(3, 31),
    APRIL(4, 30),
    MAY(5, 31),
    JUNE(6, 30),
    JULY(7, 31),
    AUGUST(8, 31),
    SEPTEMBER(9, 30),
    OCTOBER(10, 31),
    NOVEMBER(11, 30),
    DECEMBER(12, 31);

    private final int month;
    private final int days;

    Month(int month, int days) {
        this.month = month;
        this.days = days;
    }

    public static Month from(int month) {
        return Arrays.stream(Month.values())
                .filter(element -> element.month == month)
                .findFirst()
                .orElseThrow(() -> CustomException.from(ErrorMessage.INVALID_INPUT_ERROR));
    }

 

그러나 java.time.Month에서도 동일하게 사용할 수 있었으며, 특정 월의 마지막 날짜를 출력하는 기능, 숫자를 출력하는 기능 모두 내부 함수에서 정의되어있었다. 따라서 위에서 구현한 클래스를 삭제하고, 월과 관련된 요구사항의 기능을 아래와 같이 구현하였다.

// java.time.Month

   public int minLength() {
        switch (this) {
            case FEBRUARY:
                return 28;
            case APRIL:
            case JUNE:
            case SEPTEMBER:
            case NOVEMBER:
                return 30;
            default:
                return 31;
        }
    }

 
   public int getValue() {
        return ordinal() + 1;
    }

 

내가 우려하였던 부분인 윤년에 대한 부분도 minLenght()라는 함수에서 이미 구현되어있었기 때문에, 그대로 사용하기만 해도 되었었다. 😮

// java.time.Month
   public static Month of(int month) {
        if (month < 1 || month > 12) {
            throw new DateTimeException("Invalid value for MonthOfYear: " + month);
        }
        return ENUMS[month - 1];
    }

// AssignController
    private Month getMonth(int month) {
        try {
            return Month.of(month);
        } catch (DateTimeException e) {
            throw CustomException.from(ErrorMessage.INVALID_INPUT_ERROR);
        }
    }

 

특히 of 메서드를 사용해서 숫자로 입력한 월을 Month 객체로 변환할 수 있었는데, 이 과정에서 잘못된 입력 시 DateTimeException 예외가 던져졌다. 이를 AssignController에서 받아서 IllegalArgumentException을 반환하도록 수정하였다. 

 

조금 더 객체지향적으로

public class Workers {
    private Deque<Worker> workers;
	....

    /**
     * 제일 앞에 있는 요소를 반환하는 메서드
     */
    public Worker getFront() {
        return workers.getFirst();
    }

    /**
     * 제일 뒤에 있는 요소를 반환하는 메서드
     */
    public Worker getBack() {
        return workers.getLast();
    }

    /**
     * 제일 앞에 있는 요소를 제거하여 반환하는 메서드
     */
    public Worker popFront() {
        return workers.removeFirst();
    }

    /**
     * 제일 앞에 요소를 삽입하는 메서드
     */
    public void addFront(Worker worker) {
        workers.addFirst(worker);
    }

    /**
     * 제일 뒤에 요소를 삽입하는 메서드
     */
    public void addBack(Worker worker) {
        workers.addLast(worker);
    }

 

여러 근무자를 차례로 저장하는 Workers 클래스를 보자.  

여기에서 지적받은 부분은 Workers의 역할이 단순히 Deque에서 정의된 메서드를 호출하는 것에 그친다는 것이었다. 나도 구현하면서 찝찝함을 느꼈었는데, Workers라는 객체의 역할을 명확히 하고 동작을 메서드로 정의해야한다는 것이다.

 

이를 사용한 AssignManager의 기존 코드는 아래와 같다.

 private void add(Workers result, Workers workers) {
        if (result.isEmpty()) {
            addFirstWorker(result, workers);
            return;
        }
        if (isEqualsWithLastWorker(result, workers)) {
            Worker holding = workers.popFront();
            addFirstWorker(result, workers);
            workers.addFront(holding);
            return;
        }
        addFirstWorker(result, workers);
    }

    private void addFirstWorker(Workers result, Workers workers) {
        Worker worker = workers.popFront();
        result.addBack(worker);
        workers.addBack(worker);
    }

    private boolean isEqualsWithLastWorker(Workers result, Workers workers) {
        return result.getBack().equals(workers.getFront());
    }

 

Worker이 담당해야하는 역할이 AssignManager에게 넘어와서 코드가 복잡해지는 문제가 있었다.

 

여기서 Workers의 첫번째 근무자를 꺼내어 result에 저장하는 것과  result의 마지막 근무자와 새로 추가될 근무자의 동등성을 비교하는 로직을 Workers로 옮겨 보았다. 

public class Workers {
    private Deque<Worker> workers;
    
    ...

    public void addFirstWorker(Workers source) {
        Worker front = source.popFront();
        workers.addLast(front);
        source.addBack(front);
    }

    public boolean equalsWithLastWorker(Workers source) {
        Worker back = workers.getLast();
        return back.equals(source.getFront());
    }

 

이렇게 로직을 수정하면 AssignManger에서 근무자를 추가하는 메서드도 함수가 분리될 필요 없이 간단하게 만들어진다.

// AssignManager.class

   private void add(Workers result, Workers workers) {
        if (result.isEmpty()) {
            result.addFirstWorker(workers);
            return;
        }
        if (result.equalsWithLastWorker(workers)) {
            Worker holding = workers.popFront();
            result.addFirstWorker(workers);
            workers.addFront(holding);
            return;
        }
        result.addFirstWorker(workers);
    }

 

그러나 아쉬운 점은 여전히 인자로 들어온 Workers의 뒤에 요소를 추가해주거나 앞에서 요소를 제거하는 등의 연산이 필요하기 때문에, Deque를 그대로 반환하는 메서드를 만들지 않는 이상 기본적인 연산은 함수로 정의해주어야 하는 것이다. Deque에 의존된 메서드 네이밍이 아니라 많은 근무자들 중에서 일정한 방식으로 근무자를 추가하거나 제거하는 식의 네이밍을 사용하면 더 좋을 것 같았다.

 

별개의 내용이지만, 재정의 관련한 오류가 있어서 이 부분도 함께 수정하였다.

처음에 문자열 이름을 기준으로 Deque를 정의하였다가, Worker를 감싸는 컬렉션으로 수정하여 이와 같은 문제가 발생하였다. 

    @Override
    public boolean equals(Object o) {
        if (this == o) {
            return true;
        }
        if (o == null || getClass() != o.getClass()) {
            return false;
        }
        Worker worker = (Worker) o;
        return Objects.equals(name, worker.name);
    }

    @Override
    public int hashCode() {
        return Objects.hash(name);
    }

 

이렇게 Worker 객체가 이름을 기준으로 조회될 수 있도록 수정하였다. 

 

이름 길이 검증 위치에 대한 고민

 

현지님이 검증 로직에 대한 실수를 꼬집어주셨다. 

 

위와 같은 현상이 일어나게 된 이유는 뷰에서 이름을 입력 받고 뷰에 처리할 검증 로직을 처리한 후, 컨트롤러에 넘겨서 도메인을 생성하는 흐름이었기 때문이다. 결국에 이름 길이에 대한 검증이 뷰 단에서 처리되지 않기 때문에 평일 비상 근무, 휴일 비상 근무 순번을 모두 입력 받아야 컨트롤러로 넘어가서 처리되었기 떄문이다. 

    public WorkersName readWorkerInfo() {
        ConsoleWriter.printMessage("평일 비상 근무 순번대로 사원 닉네임을 입력하세요> ");
        List<String> weekday = Validator.validateWorkers(ConsoleReader.enterMessage());
        ConsoleWriter.printMessage("휴일 비상 근무 순번대로 사원 닉네임을 입력하세요> ");
        List<String> weekend = Validator.validateWorkers(ConsoleReader.enterMessage());
        return new WorkersName(
                weekday,
                weekend
        );
    }

 

이러한 경우 두 비상 근무 순번을 모두 다른 함수로 두어야 한다. 

MVC 아키텍처 패턴 원칙 상 뷰에서 Worker 도메인이 생성될 수 없기에 그대로 문자열 원시값을 넘겨주되, 두 입력값에 대한 함수를 별도로 호출하여 이 문제를 해결하였다.

    public List<String> readWeekdayWorkers() {
        ConsoleWriter.printMessage("평일 비상 근무 순번대로 사원 닉네임을 입력하세요> ");
        return Validator.validateWorkers(
                ConsoleReader.enterMessage()
        );
    }

    public List<String> readWeekendWorkers() {
        ConsoleWriter.printMessage("휴일 비상 근무 순번대로 사원 닉네임을 입력하세요> ");
        return Validator.validateWorkers(
                ConsoleReader.enterMessage()
        );
    }

retry 람다식의 메서드 분리

   private DateInfo getDateInfo() {
        return retry(() -> {
            MonthAndStartDayOfWeek monthAndStartDayOfWeek = inputView.readMonthAndStartDayOfWeek();
            Month month = getMonth(monthAndStartDayOfWeek.month());
            CustomDayOfWeek dayOfWeek = CustomDayOfWeek.from(monthAndStartDayOfWeek.startDayOfWeek());
            return new DateInfo(month, dayOfWeek);
        });
    }

 

이렇게 반복 함수인 retry를 람다식으로 사용하여 작성하였는데, 가독성을 고려하였을 때 아래와 같은 코드 스타일이 훨씬 좋은 것 같아 수정하였다. 

   DateInfo dateInfo = retry(this::getDateInfo); // 한 줄로 수정

   private DateInfo getDateInfo() {
        MonthAndStartDayOfWeek monthAndStartDayOfWeek = inputView.readMonthAndStartDayOfWeek();
        Month month = getMonth(monthAndStartDayOfWeek.month());
        CustomDayOfWeek dayOfWeek = CustomDayOfWeek.from(monthAndStartDayOfWeek.startDayOfWeek());
        return new DateInfo(month, dayOfWeek);
    }

 

 

---

이렇게 코드를 피드백하고 수정하는 시간을 거쳐 보았다. 

 

코드를 수정한 PR은 아래 링크에 있다. 

https://github.com/Mingyum-Kim/java-oncall-6-Mingyum-Kim/pull/2

[온콜] 리팩토링 코드 제출합니다. by Mingyum-Kim · Pull Request #2 · Mingyum-Kim/java-oncall-6-Mingyum-Kim

 

프리코스에서 많은 분들의 코드리뷰를 받고 이를 반영해서 개발하도록 많이 연습하였는데도 시간이 제한된 상황에서 실수하게 되는 것은 어쩔 수 없는 것 같다. 그래도 최종 코딩테스트까지 응시하여 다른 분들의 멋진 피드백을 받을 수 있음에 감사하다.  🙇‍♀️🙇‍♀️