Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

우아한테크코스 6기 최종 코딩 테스트: OnCall 리뷰용 브랜치 #1

Open
wants to merge 43 commits into
base: donghoony
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
4f9003e
docs: 기능 목록 작성
donghoony Dec 16, 2023
7b5fbf5
feat(OnCallException): 프로그램에서 사용될 커스텀 예외 템플릿 작성
donghoony Dec 16, 2023
40da02b
feat(Worker): 근무자 클래스 작성, 이름 제한 검증 구현
donghoony Dec 16, 2023
3ba3bb6
feat(OnCallRoster): 근무표 클래스 작성, 기본 검증 구현
donghoony Dec 16, 2023
5696be2
feat(OnCallRoster): 같은 근무자로 이루어졌는지 확인하는 기능 구현
donghoony Dec 16, 2023
369289a
refactor(OnCallRoster): Roster로 클래스명 변경
donghoony Dec 16, 2023
22f4a78
feat(Roster): 다음 근무자 순번을 알아내는 기능 구현
donghoony Dec 16, 2023
7a3fedc
feat(CombinedRoster): 평일, 휴일 근무표를 관리하는 기능 구현
donghoony Dec 16, 2023
07c98bd
refactor(Worker): 클래스를 레코드 타입으로 변경
donghoony Dec 16, 2023
b1a900f
feat(RosterDay): 날짜 클래스, 휴일 여부 판단 구현
donghoony Dec 16, 2023
44f2d9f
feat(RosterDay): 출력 형식에 맞도록 toString 정의
donghoony Dec 16, 2023
2364bd3
feat(MonthlyCalendar): 한 달의 달력을 구현, 날짜 수를 기반으로 날짜 생성 구현
donghoony Dec 16, 2023
6db2c73
feat(OnCallInput): 입력을 받기 위한 인터페이스 작성
donghoony Dec 16, 2023
5b157eb
feat(OnCallOutput): 출력하기 위한 인터페이스 작성
donghoony Dec 16, 2023
303702d
refactor(CombinedRoster): 직전 일한 근무자 파라미터명 변경
donghoony Dec 16, 2023
830f614
feat(DailyRoster): 일일 근무자를 기록하는 클래스 작성, toString 구현
donghoony Dec 16, 2023
ba7d9a1
feat(MonthlyRoster): 월별 근무자를 모두 담는 클래스 작성, toString 구현
donghoony Dec 16, 2023
b48d1cd
refactor(MonthlyCalendar): 해당 날짜가 속했는지 여부가 아닌, 그 달의 마지막 날을 리턴하도록 수정
donghoony Dec 16, 2023
c2a663c
feat(CombinedRoster): 날짜가 주어지면, 날짜의 휴일 여부를 판단해 다음 근무자를 배정하도록 구현
donghoony Dec 16, 2023
7e41023
feat(RosterService): 핵심 로직 구현
donghoony Dec 16, 2023
9cd3de1
style: 코드 컨벤션에 맞게 포맷 수정
donghoony Dec 16, 2023
2b176e4
refactor(OnCallOutput): 최종 근무표 출력 시, MonthlyRoster를 받도록 수정
donghoony Dec 16, 2023
64f36d7
feat(OnCallConsoleOutput): 출력 형식에 맞도록 OnCallOutput 구현체 작성
donghoony Dec 16, 2023
75c769b
refactor(OnCallInput): 평일/휴일 비상근무 입력 방식 통일
donghoony Dec 16, 2023
89d28f1
feat(OnCallConsoleInput): 입력 형식에 맞도록 OnCallInput 구현체 작성
donghoony Dec 16, 2023
931120b
refactor(RosterService): CombinedRoster를 그때그때 받아 만들 수 있도록 수정
donghoony Dec 16, 2023
ad6ad3d
feat(ExceptionLoopController): 예외 발생 시 다시 입력받을 수 있도록 하는 추상 클래스 작성
donghoony Dec 16, 2023
bf58727
refactor(RosterService): 확장성 고려해 MonthlyCalendar를 근무표 작성 시에 받도록 수정
donghoony Dec 16, 2023
31fb9a4
feat(OnCallController): 전체 플로우 로직 구현 및 조합
donghoony Dec 16, 2023
a9e12a8
fix(RosterService): 달의 마지막 날이 포함되지 않던 오류 수정
donghoony Dec 16, 2023
28d1267
fix(CombinedRoster): 다음 날짜와 체크할 때, iterator가 움직이는 현상 수정
donghoony Dec 16, 2023
b4182fc
feat(CombinedRoster): 평일 근무자와 휴일 근무자가 같도록 제약 사항 구현
donghoony Dec 16, 2023
37c7a88
test(Worker): 근무자의 이름 제한 테스트 작성
donghoony Dec 16, 2023
10b65d1
fix(OnCallController): 근무자 오입력 시 평일부터 다시 입력받도록 수정
donghoony Dec 16, 2023
c06ab0c
fix(Roster): 중복 근무자 처리 시 이터레이터 올바르게 수정
donghoony Dec 16, 2023
eeb9e20
fix(Roster): 중복 근무자 처리 시 이터레이터 올바르게 수정
donghoony Dec 16, 2023
fd11bf2
fix(Roster): 다음 순번 체크 시 정확하게 반환하도록 수정
donghoony Dec 16, 2023
f2bea11
test(Roster): 순번 뽑는 기능 테스트 작성
donghoony Dec 16, 2023
6381ab2
test(DailyRoster): 포맷 형식 테스트 작성
donghoony Dec 16, 2023
a770c3b
docs: 미션 해결 전략 작성 (1번)
donghoony Dec 16, 2023
0de20c2
docs: 미션 해결 전략 작성 (1번)
donghoony Dec 16, 2023
2412bc3
test(CombinedRoster): 검증 로직에 대한 테스트 작성
donghoony Dec 16, 2023
a92e9ab
docs: 미션 해결 전략 작성 (2번)
donghoony Dec 16, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# 우아한테크코스 최종 코딩 테스트 미션: 온콜

### 요구사항 꺼내기

대기는 순번에 따른다. 순번은 평일/주말 모두 존재하며, 각 사원은 평일과 주말에 1번 등장해야 한다.

연속 2일 대기할 수 없으며, 배치했을 때 2일이 되는 경우, 다음 날의 순번과 교체한다.

휴일은 토요일/일요일/법정공휴일이다.

2월은 28일까지 존재한다. 윤년은 고려하지 않는다.

## [도메인](#도메인)

### 사원

- [x] 이름은 5자 이내이다

### 근무표

평일 및 휴일을 같은 클래스를 사용한다

- [x] 최소 5명, 최대 35명의 사원으로 이루어져 있다
- [x] 중복되는 사람이 없어야 한다
- [x] 서로 다른 근무표에 대해, 같은 인원으로 이루어져있는지 확인한다
- [x] 다음 순번의 근무자를 알아낸다
- [x] 다다음 순번의 근무자를 알아낸다

### 평일 및 휴일 최종 근무 순번

- [x] 평일 / 휴일에 해당하는 근무 순번을 가지고 있다
- [x] 평일 / 휴일에 해당하는 다음 순번의 근무자를 알아낸다
- [x] 평일 / 휴일에 해당하는 다다음 순번의 근무자를 알아낸다

### 캘린더
- [x] 월과 첫 요일이 주어지면, 해당 월을 구축한다
- [x] 그 달의 마지막 날을 알아낸다

#### 날짜
하루를 나타내는 클래스
- [x] 출력 형식에 맞게 구성한다
- [x] 날짜가 주어지면, 해당 날짜가 휴일인지 아닌지 판단한다.

### 일일 근무자
- [x] 사원, 날짜로 이루어져 있다
- [x] 출력 형식에 맞게 구성한다

### 월간 근무자
최종적으로 사용자에게 보여질 클래스이며 서비스가 만들게 될 객체
- [x] 각 일일 근무자로 이루어져 있다
- [x] 출력 형식에 맞게 구성한다

## [로직 수행 클래스](#로직-수행-클래스)

### 인원 배치 서비스

- [x] 근무표로부터 다음 순번의 근무자를 알아낸 뒤, 배치를 시도한다.
- [x] 배치가 성공적이지 않다면, 다음 사람으로 배치한다
- [x] 직전에 근무한 근무자를 알아낸다

## [입력](#입력)

올바르지 않은 입력일 경우, `[ERROR]` 접두사와 함께 에러 메시지를 출력한다

월과 시작 요일의 값이 올바르지 않은 경우, 예외 처리한 뒤 다시 출력한다. 이때, 입력 안내 메시지도 다시 출력한다.

- [x] 비상 근무월 / 시작 요일을 입력받는다
- [x] 평일, 휴일에 해당하는 비상 근무 순번대로 사원을 입력한다

## [출력](#출력)

- [x] 입력 전, 안내 메시지를 출력한다.
- [x] 평일이면서 법정공휴일인 경우에만 휴일 표기를 한다.
- [x] 비상근무표를 출력한다.
22 changes: 22 additions & 0 deletions docs/how-to-solve.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,27 @@
### 미션 해결 전략
#### 1. 본인이 이해하고 구현한 내용에 기반해 '다른 근무자와 순서를 바꿔야 하는 경우'를 자신만의 예시를 들어 설명하세요. (필수)

다른 근무자와 순서를 바꿔야 하는 경우는 다음과 같다
- 바로 **직전에 배치한 근무자**와, **현재 배치하려는 근무자**가 같은 경우

이는 평일-평일, 휴일-휴일에서는 나타나지 않는다. 요구사항에서 평일/휴일 명단을 불러올 때, 중복되는 사람이 존재하지 않았기 때문이다.

따라서 평일-휴일, 휴일-평일에서만 생각해도 충분하고, 직전에 배치한 근무자를 따로 저장해 현재 근무자를 확인하도록 설계했다.

다음 근무자를 배정하는 과정을 간단하게 생각한다면, 근무를 했는지 여부를 나타내는 배열/리스트를 두고 순회하는 방법이 있다. 하지만 이 경우, 근무자가 많아짐에 따라 조회에 오랜 시간이 걸리게 된다.

계산상의 이점을 가져갈 수 없다고 생각했고, 따라서 `Collections`에서 제공하는 `Iterator`를 사용해 보다 효율적으로 탐색했다.

`Iterator`는 선형이기에, 요구사항에서 원하는 원형 탐색을 구현해야 했다. `hasNext` 또는 `hasPrevious`가 거짓인 경우 그 반대쪽을 가리키도록 해서 요구사항을 만족시켰다.

만약 이틀 연속 근무하는 상황이 발생해 근무를 바꿔야하는 경우, 그 날의 속성(평일/휴일)에만 영향을 미친다. 즉, 평일-휴일에서 휴일에 근무를 두 번 하게 되는 경우, 휴일만 확인하면 된다. 따라서 해당 명단만 확인했고, 다음 근무자를 선정한 뒤 `skipped` 플래그를 통해 다음에 이미 뽑힌 근무자를 지나쳐 가야 하는지 여부를 나타냈다.

#### 2. 요구사항에서 제시한 앞의 날짜부터 순서를 변경하는 방법 외에 다른 방법이 있다면 어떤 방식이 있는지, 이 방법은 기존에 제시된 방식과 비교해 어떤 차이가 있는지 설명하세요. (선택)

근무표를 작성할 때, 가장 중요한 것은 아무래도 공정성일 것이다. 무작위를 통해 근무를 선정한다면 한 사람이 다른 사람들에 비해 더 많이 근무할 수도 있어 이상적이지 않다.

두 번 연속 근무하게 돼 다음 순번과 바꾸는 것은, 다음 사람에 대한 공정성을 해치는 것이기도 하다. 자신이 이 날 근무할 것이라고 예상했지만, 두 번 연속으로 근무하게 될 사람으로 인해 예상치 못하게 근무할 날들이 존재할 수 있기 때문이다.

따라서 타인에 의해 근무가 바뀐 사람의 횟수를 센 뒤, 일정 값이 넘는다면 그 사람과는 바꾸지 않고, 그 다음 사람과 바꾸는 과정을 진행하면 더 공정하게 될 것이다.

이렇게 된다면 최악의 경우 한 사람만 계속해서 근무를 바꿔주는 상황을 최소화할 수 있고, 모두가 공평하다고 생각하는 근무표가 작성될 것이다.
15 changes: 14 additions & 1 deletion src/main/java/oncall/Application.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,20 @@
package oncall;

import oncall.controller.OnCallController;
import oncall.io.OnCallConsoleInput;
import oncall.io.OnCallConsoleOutput;
import oncall.io.OnCallInput;
import oncall.io.OnCallOutput;
import oncall.service.RosterService;

public class Application {
public static void main(String[] args) {
// TODO: 프로그램 구현
OnCallInput input = new OnCallConsoleInput();
OnCallOutput output = new OnCallConsoleOutput();

RosterService rosterService = new RosterService();
OnCallController controller = new OnCallController(rosterService, input, output);

controller.makeRoster();
}
}
20 changes: 20 additions & 0 deletions src/main/java/oncall/config/HolidayConfig.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package oncall.config;

import java.time.MonthDay;
import java.util.Set;

public class HolidayConfig {
public static final Set<MonthDay> holidays = Set.of(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HolidayConfig.holidays.contains() 이런식으로 사용하고 계신대 이 Set을 private으로 변경하고, contains() 메서드를 여기 HolidayConfig 클래스 내부로 옮기는 형태는 어떤가요?
그러면 좀더 메시지를 주고 받는 형태가 될 수 있을 거 같아요!

아니면 class 내부에 Set이 있는 형태가 아닌 enum 에 변수 형태도 좋을 거 같아요!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네 이건 Enum으로 만들어서 메서드를 여는 게 현명했을 것 같습니다. 5시간 안에 짜다 보니 쉽게 생각하는 방향으로 코드가 짜졌네요, 코멘트 고맙습니다!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 이 부분을 RosterDay 내부에서 직접 상수로 가지고 있는 것이 좋을 것 같다는 코멘트를 드리려 왔습니다.
결국 "날짜가 법정 공휴일인지 판단" 하는 책임은 날짜를 나타내는 RosterDay의 책임이고, 그렇기에 법정 공휴일의 정보 역시 RosterDay가 가지고 있는 것이 응집도가 높아진다고 생각했기 때문이에요.

다만, 그렇게 될 경우 RosterDay의 책임이 과하게 커질 수 있을 것 같기에 취향의 영역이라 생각 됩니다!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

처음에는 저도 RosterDay 객체에 두었다가, 객체 책임이 커지는데다 이 부분은 요구자에 의해 쉽게 변동되는 부분이라 Config라는 개념을 생각했습니다.

조금 더 다듬었으면 좋겠다라는 생각은 계속 드네요. 사실 4주 차 프리코스에서도 할인 정책을 클래스로 만들지 않고, Config로 접근해 해당 클래스에서 할인 정책들을 직접 구현하는 방식으로 진행했거든요.

시간이 조금 더 있었더라면.. 이라는 아쉬움이 남네요 😂

MonthDay.of(1, 1),
MonthDay.of(3, 1),
MonthDay.of(5, 5),
MonthDay.of(6, 6),
MonthDay.of(8, 15),
MonthDay.of(10, 3),
MonthDay.of(10, 9),
MonthDay.of(12, 25)
);

private HolidayConfig() {
}
}
16 changes: 16 additions & 0 deletions src/main/java/oncall/controller/ExceptionLoopController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package oncall.controller;

import java.util.function.Supplier;
import oncall.exception.OnCallException;

public abstract class ExceptionLoopController {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 클래스가 추상 클래스일 필요는 없을 것 같습니다!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아래에 답변하겠습니다 😁

protected <T> T repeatUntilValid(Supplier<T> function) {
while (true) {
try {
return function.get();
} catch (OnCallException e) {
System.out.println(e.getMessage());
}
}
}
}
55 changes: 55 additions & 0 deletions src/main/java/oncall/controller/OnCallController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package oncall.controller;

import java.util.List;
import oncall.domain.CombinedRoster;
import oncall.domain.MonthlyCalendar;
import oncall.domain.MonthlyRoster;
import oncall.domain.Roster;
import oncall.domain.Worker;
import oncall.io.OnCallInput;
import oncall.io.OnCallOutput;
import oncall.service.RosterService;

public class OnCallController extends ExceptionLoopController {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ControllerExceptionLoopController를 "상속" 받도록 구현하신 이유가 있으실까요?
상속 이외에도 ExceptionLoopController를 필드로 가지는 조합이나 인터페이스로 구현하는 방법 등 다양한 방법이 있었을텐데, 상속을 선택하신 이유가 궁금해요

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

처음에는 default 메서드를 가지는 인터페이스를 가지고 구현해 두었습니다만, 추상 클래스로 만든 데에는 몇 가지 이유가 있었습니다

  • ExceptionLoopController가 그 자체로 생성되는 것을 방지하고자 했습니다
  • 추후 다른 함수가 추가되는 경우 ExceptionLoopController만 수정하면 다른 곳에서도 관련 메서드를 사용할 수 있게 됩니다. 이 경우 인터페이스의 성질보다는 추상 클래스의 성질이 더 강하다고 보았어요.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아, 추상 클래스로 작성하신 것은 인스턴스화 방지의 이유가 있었군요.
다만, 여전히 저라면 생성자를 private으로 숨기는 방법으로 인스턴스화를 방지할 것 같습니다.
추상 클래스는 내부에 추상 메서드를 포함하여 다형성을 제공하기 위해 사용하는 것이라고 생각하기 때문에요!


저 역시 프리코스 기간동안 상속을 사용하면서 정말 많은 피드백을 받았는데요. 상속이라는 개념은 알다가도 모를 것 같습니다...ㅎㅎ

아시겠지만, 상속을 사용하게 되면 부모 클래스와 자식 클래스가 강하게 결합해 변경이 힘들다는 단점이 존재합니다.
예를 들어 (물론 당연히 그럴 일은 없겠지만) 갑자기 최종 코테의 요구사항이 변경되어 예외가 발생하면 다시 입력을 받지 않고, 그대로 예외를 출력하며 종료해야 한다고 가정해 보겠습니다.
이 때 상속을 사용한 Controller의 경우는 부모 클래스 자체를 ExceptionLoopController가 아닌 ExceptionQuitController를 상속하도록 변경해야 할 것 같습니다.

현재 예외 처리 방법이라는 책임은 OnCallController에서 분리되어 있기에 다른 요구사항 변경에 OncallController의 코드 변경이 필요해 질 것이라 생각해요.

이런 상황에서 만약

  1. 예외 처리 방법이 ExceptionHandler라는 인터페이스로 제공되고 있었고,
  2. ExceptionHandler의 구현 클래스가 외부에서 주입되고 있었다

라고 가정해 보겠습니다.

이 때는 외부에서 주입해 주는 ExceptionHandler의 구현 클래스를 변경시켜 주는 것 만으로 요구사항 변경에 대처할 수 있게 될 것 같습니다.


물론, 코테를 구현하는 동안은 해당 부분에 대한 변경이 일어나지 않을 것이기에 오버 엔지니어링이 될 것 같기도 합니다. (저 역시 이런 이유로 ExceptionHandler에 추상화를 제공하지 않은 채 직접 구현 클래스를 사용했어요)
다만, 저 역시 상속을 사용하는 기준에 많은 고민을 가지고 있는 만큼 이에 대해 의견을 많이 나눠보고 싶었어요 ㅎㅎ

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

상속이 부모-자식 간 강결합이 발생한다는 것은 크게 동감해요, 그래서 저도 상속은 되도록이면 (꼭 필요한 게 아니라면) 피하고 있습니다. 사실 꼭 필요하다는 것도 아직은 잘 모르겠습니다... 언제 상속을 사용해야 좋을까요?
이번 프리코스를 진행하면서 상속을 쓸 일이 크게 없었고, 상속을 쓸 상황에서도 인터페이스를 위주로 사용한 터라 어떤 식으로 사용하는 지 감이 잘 오지를 않네요 🤔

인터페이스가 있다면, 추상 클래스는 존재하지 않아도 된다는 것일까요? 강결합이긴 하지만, 실제로 요구사항에서 강결합이 필요하다면 추상 클래스를 사용해야 하는 걸까요?

너무 어려운 주제같습니다 ㅎㅎ 🥵

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저 역시 이 부분이 너-무 어려운데요...ㅎㅎ
최종 코테 대비를 함께 하시던 스터디원중 한 분은 "어떤 기술을 사용하기 위한 조건을 확인하는 과정이 많이 번거롭다면 해당 기술을 배제하고 대체제를 사용하는 편이다." 라는 의견을 주셨던 것이 기억납니다!!

확실한 is-a 관계를 파악하는 것이 제일 핵심일 것 같은데요.
상속으로 구현할 수 있는 상황의 대부분은 인터페이스를 사용한 전략패턴 등으로 손쉽게 대체가 가능하기에 더더욱 상속이 적절한 상황을 찾기 어려워 지는 것 같습니다...ㅎㅎ

private final RosterService rosterService;
private final OnCallInput input;
private final OnCallOutput output;

public OnCallController(RosterService rosterService, OnCallInput input, OnCallOutput output) {
this.rosterService = rosterService;
this.input = input;
this.output = output;
}


public void makeRoster() {
MonthlyCalendar monthlyCalendar = repeatUntilValid(this::makeMonthlyCalendar);

CombinedRoster combinedRoster = repeatUntilValid(this::makeCombinedRoster);
MonthlyRoster monthlyRoster = rosterService.makeMonthlyRoster(monthlyCalendar, combinedRoster);
output.printRoster(monthlyRoster);
}

private MonthlyCalendar makeMonthlyCalendar() {
output.printAskingMonthAndWeekDay();
return input.getMonthAndDayOfWeek();
}

private CombinedRoster makeCombinedRoster() {
Roster weekDayRoster = makeWeekDayRoster();
Roster holidayRoster = makeHolidayRoster();
return new CombinedRoster(weekDayRoster, holidayRoster);
}

private Roster makeWeekDayRoster() {
output.printAskingWeekDayWorkers();
List<Worker> weekdayWorkers = input.getWorkers();
return new Roster(weekdayWorkers);
}

private Roster makeHolidayRoster() {
output.printAskingHolidayWorkers();
List<Worker> holidayWorkers = input.getWorkers();
return new Roster(holidayWorkers);
}
}
44 changes: 44 additions & 0 deletions src/main/java/oncall/domain/CombinedRoster.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package oncall.domain;

import oncall.exception.IllegalCombinedRosterException;

public class CombinedRoster {
private final Roster weekdayRoster;
private final Roster holidayRoster;

public CombinedRoster(Roster weekdayRoster, Roster holidayRoster) {
validateIdenticalWorkers(weekdayRoster, holidayRoster);
this.weekdayRoster = weekdayRoster;
this.holidayRoster = holidayRoster;
}


public Worker getWorker(RosterDay rosterDay, Worker previousWorker) {
if (rosterDay.isHoliday()) {
return getHolidayWorker(previousWorker);
}
return getWeekdayWorker(previousWorker);
}

private void validateIdenticalWorkers(Roster weekdayRoster, Roster holidayRoster) {
if (!weekdayRoster.isSameAs(holidayRoster)) {
throw new IllegalCombinedRosterException();
}
}
Comment on lines +23 to +27

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기에서 길이가 다른 동일하지 않은 근무자일 경우 체크가 되지 않고 있어요..!

평일: a,b,c,d,e
공휴일: a,b,c,d,e,f

isSameAs 메서드를 보니 평일과 공휴일에 같은 요소를 센다음 평일 길이랑 비교하고 있는 형태더라구요

 if (!weekdayRoster.isSameAs(holidayRoster) || !holidayRoster.isSameAs(weekdayRoster)))

이렇게 사용해도 좋을 거 같아요!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

생각지도 못한 부분이네요, 짚어주셔서 정말 고맙습니다. isSameAs를 좀 더 구체화하는 방향으로 짰어야 했네요


private Worker getWeekdayWorker(Worker previousWorker) {
Worker worker = weekdayRoster.checkNextWorker();
if (worker.equals(previousWorker)) {
return weekdayRoster.getAfterNextWorker();
}
return weekdayRoster.getNextWorker();
}

private Worker getHolidayWorker(Worker previousWorker) {
Worker worker = holidayRoster.checkNextWorker();
if (worker.equals(previousWorker)) {
return holidayRoster.getAfterNextWorker();
}
return holidayRoster.getNextWorker();
}
Comment on lines +29 to +43

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

평일, 휴일 두 경우에 따라 메서드가 나뉘어서 작성되어 있는데요,
if (rosterDay.isHoliday()) 조건문 확인후 weekdayRoster, holidayRoster를 매개변수로 넘겨준다면 하나의 메서드로 두 경우에 맞게 처리할 수 있을 것 같습니다.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이건 코드를 짤 때에도 고민을 많이 했습니다. how-to-solve에서 다른 전략이 들어왔을 때의 확장성을 고려해서, 각각의 전략은 (지금은 같지만) 다른 메서드로 구현해 두었어요.

}
16 changes: 16 additions & 0 deletions src/main/java/oncall/domain/DailyRoster.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package oncall.domain;

public class DailyRoster {
private final RosterDay rosterDay;
private final Worker worker;

public DailyRoster(RosterDay rosterDay, Worker worker) {
this.rosterDay = rosterDay;
this.worker = worker;
}

@Override
public String toString() {
return rosterDay.toString() + " " + worker.name();
}
Comment on lines +12 to +15

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

도메인의 toString() 메서드가 뷰의 출력을 담당하는 것이 패턴의 원칙에 어긋난다고 생각합니다. 어떻게 생각하시는 지 궁금해요!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DailyRoster는 메서드가 없는 하나의 값 객체로 보아서, 해당 객체의 표현 방식을 toString으로 나타내는 것은 자연스럽다고 보았습니다. 3주 차 피드백의 현재 객체의 상태를 보기 위한 로그 메시지 성격이 강하다면 toString()을 통해 구현한다. 에서 가져왔습니다!

}
23 changes: 23 additions & 0 deletions src/main/java/oncall/domain/MonthlyCalendar.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package oncall.domain;

import java.time.DayOfWeek;
import java.time.Month;
import java.time.MonthDay;

public class MonthlyCalendar {
private final Month month;
private final DayOfWeek startDayOfWeek;

public MonthlyCalendar(Month month, DayOfWeek startDayOfWeek) {
this.month = month;
this.startDayOfWeek = startDayOfWeek;
}

public RosterDay get(int day) {
return new RosterDay(MonthDay.of(month, day), startDayOfWeek.plus(day - 1L));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

자바에서 제공하는 기본 API를 정말 잘 활용하신 것 같습니다 👍

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 startDayOfWeek.plus 이런 부분 처럼
저는 수제 코딩(?)으로 했던 부분을 이렇게 구현하신 걸 보고 '저런 게 있었구나ㅠㅠ'하고 갑니다ㅎㅎ

}
Comment on lines +16 to +18

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

단순히 get 메서드를 이름으로 사용하는 것이 외부에서 어떤 값을 리턴하는 지 모호한 것 같아요.
차라리 정적 팩토리 메서드를 사용해서 from을 정의하는 것이 컨벤션과 더 일치하다고 생각합니다!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

정적 팩토리로 가져오는 편이 나았겠습니다. 뒤돌아볼 틈이 없었네요! 고맙습니다 ㅎㅎ


public int getLastDay() {
return month.minLength();
}
}
22 changes: 22 additions & 0 deletions src/main/java/oncall/domain/MonthlyRoster.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package oncall.domain;

import java.util.List;
import java.util.StringJoiner;

public class MonthlyRoster {
private final List<DailyRoster> dailyRosters;

public MonthlyRoster(List<DailyRoster> dailyRosters) {
this.dailyRosters = dailyRosters;
}

@Override
public String toString() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toString()을 이용해 출력 메시지를 처리해 주신 것 같아요.
이 부분에 대해선 정말 다양한 의견이 있는데, 많은 사람이 toString() 메서드는 디버깅을 위한 용도임을 주장합니다.
관련 아티클 하나 첨부 드립니다. Java 에서 toString 메소드의 올바른 사용 용도에 대하여

물론 이는 다양한 의견 중 하나일 뿐이기에 고민해 보시고 적절한 방법을 찾으시면 좋을 것 같습니다.

+) 다만, 도메인 객체가 출력의 책임을 가지게 된다면 출력 형식의 변화가 생겼을 때 일반적인 출력의 기능을 제공하는 view 계층이 아닌 도메인 로직을 수정하게 됩니다. 때문에 도메인 객체에서 출력의 책임을 분리해 내기도 하는 것으로 알고 있습니다.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

도메인 객체에서 출력을 담당하기는 하지만, 해당 객체가 다른 메서드 없이 단순히 값만을 제공하는 객체이기 때문에 toString을 통해서 표현했습니다. 실제 로직을 수행하는 RosterCombinedRoster와 같은 객체에는 toString이 없는 것이 그 이유예요.

해당 객체에서 get을 사용해서 뽑을 수도 있었겠지만, 그렇게 되면 또 객체에게 메시지를 던지는 느낌보다는 직접 속성을 뽑아서 출력하는 느낌이 들어서, 어떻게 보면 출력을 위한 객체라고 할 수도 있을 것 같습니다.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아하, 일종의 DTO, 또는 VO와 비슷한 역할을 구현하시려고 했던 것 같아요!
이해가 되었습니다!!

StringJoiner stringJoiner = new StringJoiner(System.lineSeparator());
dailyRosters.stream()
.map(DailyRoster::toString)
.forEach(stringJoiner::add);

return stringJoiner.toString();
}
}
Loading