우아한테크코스 6기 최종 코딩 테스트: OnCall 리뷰용 브랜치 #1
Conversation
앞으로 예외 사항은 해당 클래스를 상속하여 진행한다
기본적인 근무자 클래스를 작성하고, 이에 대한 이름 검증 클래스를 구현한다. 이름은 최대 다섯 글자 이내여야 한다
근무자로 이루어진 근무표 클래스를 작성하고, 기본 검증을 구현한다. 근무자가 5명 이상 35명 이하여야 하고, 중복된 근무자는 존재하면 안 된다.
두 개의 근무표를 비교해 같은 근무자로 이루어졌는지 확인하는 기능을 구현한다
어플리케이션 이름이 OnCall이므로, 불필요한 접두사를 제거한다
`Iterator`를 사용해 다음 근무자 순번을 알아낸다. 경우에 따라 다다음 근무자 순번이 필요할 수 있으므로, 이 경우에는 다다음을 얻은 뒤, 스킵을 만들어 둔다.
평일, 휴일에 다음 순번을 가져오는 메서드를 가진다. 직전 근무자 정보를 받아, 다음 혹은 다다음 순번의 근무자를 알려주는 역할을 한다
Worker는 이름만을 가진다. 또 이름만으로 다른 사람과 비교하므로, 간결하게 작성하기 위해 Record로 변경한다
기존 날짜 클래스는 연도가 필요하기에, 적절한 날짜를 만들기 위해 새로운 자료형이 필요하다고 판단한다. 따라서 월, 일과 그날의 요일을 받는다. 또, 해당 날짜가 휴일에 포함되는지 확인한다
`Locale`을 활용해 원하는 대로 출력할 수 있게 구현한다.
`RosterDay`를 리턴해 주는 클래스. 프로그램은 한 달 안에서 돌아가기 때문에, 해당 달을 한 번 생성한 뒤 계속 사용한다
Service에서 생성할 일일 근무자 클래스, 각각의 toString과 worker.name을 통해 출력 형식을 지킨다
Service에서 생성할 월별 근무자 클래스. 최종적으로 서비스가 만들어 내는 것이며, 이를 출력하여 사용자에게 보여준다
바깥에서 각각 `getWeekDayWorker`, `getHolidayWorker`를 분기하지 않고, 내부에서 확인해 알아내도록 한다
지금까지 작성한 도메인을 바탕으로 특정 달의 근무표를 만들어내는 역할을 한다.
두 개의 입력 메서드가 아닌, 같은 메서드를 활용하도록 한다. 추후에 컨트롤러 단에서 이를 적절히 활용한다
입력 사항 및 입력에 대한 검증을 이곳에서 수행한다
확장성 및 재사용성을 고려해서, 서비스가 생성될 때 고정하는 것이 아니라, 매번 근무표를 생성할 때마다 명단을 받아오도록 수정한다
IntStream.range`는 exclusive이기 때문에 끝 날짜가 포함되지 않았다. 이를 `rangeClosed`로 수정해 구현한다
`getNextWorker`로 확인할 때 `iterator`가 같이 움직여 다다음 근무자가 선정되던 오류를 수정한다
| public class IllegalRoasterSizeException extends OnCallException { | ||
|
|
||
| public IllegalRoasterSizeException() { | ||
| super("근무표 "); |
There was a problem hiding this comment.
에러 메시지를 온전하게 작성하지 못했네요 😢
| 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(); | ||
| } |
There was a problem hiding this comment.
평일, 휴일 두 경우에 따라 메서드가 나뉘어서 작성되어 있는데요,
if (rosterDay.isHoliday()) 조건문 확인후 weekdayRoster, holidayRoster를 매개변수로 넘겨준다면 하나의 메서드로 두 경우에 맞게 처리할 수 있을 것 같습니다.
There was a problem hiding this comment.
이건 코드를 짤 때에도 고민을 많이 했습니다. how-to-solve에서 다른 전략이 들어왔을 때의 확장성을 고려해서, 각각의 전략은 (지금은 같지만) 다른 메서드로 구현해 두었어요.
| private static final int MAX_WORKERS = 35; | ||
|
|
||
| private final List<Worker> workers; | ||
| private ListIterator<Worker> iterator; |
There was a problem hiding this comment.
Iterator를 사용하여 선형구조로 탐색할 수 있도록 로직을 구현하신 점이 정말 멋집니다.
There was a problem hiding this comment.
처음에는 이것밖에 생각을 못 했는데, deque 등 다양한 자료구조를 사용하셨더라고요!
| import oncall.config.HolidayConfig; | ||
|
|
||
| public class RosterDay { | ||
| private static final DateTimeFormatter FORMAT_PATTERN = DateTimeFormatter.ofPattern("M월 d일"); |
There was a problem hiding this comment.
DateTimeFormatter라는 것도 있군요. 새로 배워갑니다.
| import java.util.Set; | ||
|
|
||
| public class HolidayConfig { | ||
| public static final Set<MonthDay> holidays = Set.of( |
There was a problem hiding this comment.
HolidayConfig.holidays.contains() 이런식으로 사용하고 계신대 이 Set을 private으로 변경하고, contains() 메서드를 여기 HolidayConfig 클래스 내부로 옮기는 형태는 어떤가요?
그러면 좀더 메시지를 주고 받는 형태가 될 수 있을 거 같아요!
아니면 class 내부에 Set이 있는 형태가 아닌 enum 에 변수 형태도 좋을 거 같아요!
There was a problem hiding this comment.
네 이건 Enum으로 만들어서 메서드를 여는 게 현명했을 것 같습니다. 5시간 안에 짜다 보니 쉽게 생각하는 방향으로 코드가 짜졌네요, 코멘트 고맙습니다!
There was a problem hiding this comment.
저는 이 부분을 RosterDay 내부에서 직접 상수로 가지고 있는 것이 좋을 것 같다는 코멘트를 드리려 왔습니다.
결국 "날짜가 법정 공휴일인지 판단" 하는 책임은 날짜를 나타내는 RosterDay의 책임이고, 그렇기에 법정 공휴일의 정보 역시 RosterDay가 가지고 있는 것이 응집도가 높아진다고 생각했기 때문이에요.
다만, 그렇게 될 경우 RosterDay의 책임이 과하게 커질 수 있을 것 같기에 취향의 영역이라 생각 됩니다!
There was a problem hiding this comment.
처음에는 저도 RosterDay 객체에 두었다가, 객체 책임이 커지는데다 이 부분은 요구자에 의해 쉽게 변동되는 부분이라 Config라는 개념을 생각했습니다.
조금 더 다듬었으면 좋겠다라는 생각은 계속 드네요. 사실 4주 차 프리코스에서도 할인 정책을 클래스로 만들지 않고, Config로 접근해 해당 클래스에서 할인 정책들을 직접 구현하는 방식으로 진행했거든요.
시간이 조금 더 있었더라면.. 이라는 아쉬움이 남네요 😂
|
|
||
| @Override | ||
| public void printAskingHolidayWorkers() { | ||
| System.out.println("휴일 비상 근무 순번대로 사원 닉네임을 입력하세요> "); |
There was a problem hiding this comment.
여기도 println 대신 print 를 사용하면 좋을 거 같군요!
There was a problem hiding this comment.
어쩐지 계속 커서가 내려간다 했는데,, 여기서 개행을 하고 있었네요 😢
| private void validateIdenticalWorkers(Roster weekdayRoster, Roster holidayRoster) { | ||
| if (!weekdayRoster.isSameAs(holidayRoster)) { | ||
| throw new IllegalCombinedRosterException(); | ||
| } | ||
| } |
There was a problem hiding this comment.
여기에서 길이가 다른 동일하지 않은 근무자일 경우 체크가 되지 않고 있어요..!
평일: a,b,c,d,e
공휴일: a,b,c,d,e,f
isSameAs 메서드를 보니 평일과 공휴일에 같은 요소를 센다음 평일 길이랑 비교하고 있는 형태더라구요
if (!weekdayRoster.isSameAs(holidayRoster) || !holidayRoster.isSameAs(weekdayRoster)))이렇게 사용해도 좋을 거 같아요!
There was a problem hiding this comment.
생각지도 못한 부분이네요, 짚어주셔서 정말 고맙습니다. isSameAs를 좀 더 구체화하는 방향으로 짰어야 했네요
| import oncall.exception.DuplicateWorkerException; | ||
| import oncall.exception.IllegalRoasterSizeException; | ||
|
|
||
| public class Roster { |
There was a problem hiding this comment.
이번 미션에서 핵심 부분을 담당하고 있는 클래스인거 같은데 메서드 분리가 깔끔하군요👍
| @Override | ||
| public String toString() { | ||
| String represent = monthDay.format(FORMAT_PATTERN) + " " + | ||
| dayOfWeek.getDisplayName(TextStyle.SHORT, Locale.KOREA); |
| @Override | ||
| public String toString() { | ||
| return rosterDay.toString() + " " + worker.name(); | ||
| } |
There was a problem hiding this comment.
도메인의 toString() 메서드가 뷰의 출력을 담당하는 것이 패턴의 원칙에 어긋난다고 생각합니다. 어떻게 생각하시는 지 궁금해요!
There was a problem hiding this comment.
DailyRoster는 메서드가 없는 하나의 값 객체로 보아서, 해당 객체의 표현 방식을 toString으로 나타내는 것은 자연스럽다고 보았습니다. 3주 차 피드백의 현재 객체의 상태를 보기 위한 로그 메시지 성격이 강하다면 toString()을 통해 구현한다. 에서 가져왔습니다!
| public RosterDay get(int day) { | ||
| return new RosterDay(MonthDay.of(month, day), startDayOfWeek.plus(day - 1L)); | ||
| } |
There was a problem hiding this comment.
단순히 get 메서드를 이름으로 사용하는 것이 외부에서 어떤 값을 리턴하는 지 모호한 것 같아요.
차라리 정적 팩토리 메서드를 사용해서 from을 정의하는 것이 컨벤션과 더 일치하다고 생각합니다!
There was a problem hiding this comment.
정적 팩토리로 가져오는 편이 나았겠습니다. 뒤돌아볼 틈이 없었네요! 고맙습니다 ㅎㅎ
| if (!dayOfWeekMap.containsKey(s)) { | ||
| throw new IllegalInputException(); | ||
| } | ||
| return dayOfWeekMap.get(s); |
There was a problem hiding this comment.
저는 "Map에서 값을 조회하고, 값이 없다면 예외를 발생한다." 라는 로직이 하나의 값 조회 과정이라고 생각해요.
때문에 이 로직을 하나의 흐름으로 표현할 수 있다면 그렇게 표현하려고 노력합니다!
| if (!dayOfWeekMap.containsKey(s)) { | |
| throw new IllegalInputException(); | |
| } | |
| return dayOfWeekMap.get(s); | |
| return Optional.ofNullable(dayOfWeekMap.get(s)) | |
| .orElseThrow(IllegalInputException::new); |
There was a problem hiding this comment.
오 좋은데요? 두 단계의 흐름이 아닌 하나의 흐름으로 로직을 가져가는 게 인상깊습니다. 조언 고맙습니다 ❤️
| private static final String DELIMITER = ","; | ||
|
|
||
| @Override | ||
| public MonthlyCalendar getMonthAndDayOfWeek() { |
There was a problem hiding this comment.
view 계층에서 도메인 객체를 직접 생성하여 반환하도록 구현하신 것은 시간제약에 따른 타협 부분일까요?
There was a problem hiding this comment.
참 어렵네요 😢 사실 저는 4주동안 진행하면서 MVC 패턴에 대해서는 잘 몰랐고, 객체지향의 관점에서 계속해서 생각하다 보니 MVC와 같이 IO/Service/Domain/Controller로 나뉘게 되었습니다.
사실 제 생각의 흐름은 Input에서 객체를 만들 때, 어차피 Domain단에서 예외가 발생하게 되므로 객체를 반환해도 문제가 없다는 것이었는데요, View가 Domain이 어떻게 구성돼 있는지를 구체적으로 의존하고 있는 듯 하기도 하고요. 공부거리가 늘었습니다 ✏️
There was a problem hiding this comment.
아, 저는 웹 MVC를 알고 계신다는 가정 하에 드렸던 리뷰입니다!
아직 MVC 패턴이 중심 학습거리가 될 필요는 없을 것 같습니다!!! (저 역시 MVC 패턴이 웹 개발에서라면 모를까, 콘솔 환경에서는 그렇게까지 필요하다고 느껴지진 않았습니다.)
| } | ||
|
|
||
| public RosterDay get(int day) { | ||
| return new RosterDay(MonthDay.of(month, day), startDayOfWeek.plus(day - 1L)); |
There was a problem hiding this comment.
저도 startDayOfWeek.plus 이런 부분 처럼
저는 수제 코딩(?)으로 했던 부분을 이렇게 구현하신 걸 보고 '저런 게 있었구나ㅠㅠ'하고 갑니다ㅎㅎ
| import java.util.function.Supplier; | ||
| import oncall.exception.OnCallException; | ||
|
|
||
| public abstract class ExceptionLoopController { |
| import oncall.io.OnCallOutput; | ||
| import oncall.service.RosterService; | ||
|
|
||
| public class OnCallController extends ExceptionLoopController { |
There was a problem hiding this comment.
Controller가 ExceptionLoopController를 "상속" 받도록 구현하신 이유가 있으실까요?
상속 이외에도 ExceptionLoopController를 필드로 가지는 조합이나 인터페이스로 구현하는 방법 등 다양한 방법이 있었을텐데, 상속을 선택하신 이유가 궁금해요
There was a problem hiding this comment.
처음에는 default 메서드를 가지는 인터페이스를 가지고 구현해 두었습니다만, 추상 클래스로 만든 데에는 몇 가지 이유가 있었습니다
ExceptionLoopController가 그 자체로 생성되는 것을 방지하고자 했습니다- 추후 다른 함수가 추가되는 경우
ExceptionLoopController만 수정하면 다른 곳에서도 관련 메서드를 사용할 수 있게 됩니다. 이 경우 인터페이스의 성질보다는 추상 클래스의 성질이 더 강하다고 보았어요.
There was a problem hiding this comment.
아, 추상 클래스로 작성하신 것은 인스턴스화 방지의 이유가 있었군요.
다만, 여전히 저라면 생성자를 private으로 숨기는 방법으로 인스턴스화를 방지할 것 같습니다.
추상 클래스는 내부에 추상 메서드를 포함하여 다형성을 제공하기 위해 사용하는 것이라고 생각하기 때문에요!
저 역시 프리코스 기간동안 상속을 사용하면서 정말 많은 피드백을 받았는데요. 상속이라는 개념은 알다가도 모를 것 같습니다...ㅎㅎ
아시겠지만, 상속을 사용하게 되면 부모 클래스와 자식 클래스가 강하게 결합해 변경이 힘들다는 단점이 존재합니다.
예를 들어 (물론 당연히 그럴 일은 없겠지만) 갑자기 최종 코테의 요구사항이 변경되어 예외가 발생하면 다시 입력을 받지 않고, 그대로 예외를 출력하며 종료해야 한다고 가정해 보겠습니다.
이 때 상속을 사용한 Controller의 경우는 부모 클래스 자체를 ExceptionLoopController가 아닌 ExceptionQuitController를 상속하도록 변경해야 할 것 같습니다.
현재 예외 처리 방법이라는 책임은 OnCallController에서 분리되어 있기에 다른 요구사항 변경에 OncallController의 코드 변경이 필요해 질 것이라 생각해요.
이런 상황에서 만약
- 예외 처리 방법이
ExceptionHandler라는 인터페이스로 제공되고 있었고, ExceptionHandler의 구현 클래스가 외부에서 주입되고 있었다
라고 가정해 보겠습니다.
이 때는 외부에서 주입해 주는 ExceptionHandler의 구현 클래스를 변경시켜 주는 것 만으로 요구사항 변경에 대처할 수 있게 될 것 같습니다.
물론, 코테를 구현하는 동안은 해당 부분에 대한 변경이 일어나지 않을 것이기에 오버 엔지니어링이 될 것 같기도 합니다. (저 역시 이런 이유로 ExceptionHandler에 추상화를 제공하지 않은 채 직접 구현 클래스를 사용했어요)
다만, 저 역시 상속을 사용하는 기준에 많은 고민을 가지고 있는 만큼 이에 대해 의견을 많이 나눠보고 싶었어요 ㅎㅎ
There was a problem hiding this comment.
상속이 부모-자식 간 강결합이 발생한다는 것은 크게 동감해요, 그래서 저도 상속은 되도록이면 (꼭 필요한 게 아니라면) 피하고 있습니다. 사실 꼭 필요하다는 것도 아직은 잘 모르겠습니다... 언제 상속을 사용해야 좋을까요?
이번 프리코스를 진행하면서 상속을 쓸 일이 크게 없었고, 상속을 쓸 상황에서도 인터페이스를 위주로 사용한 터라 어떤 식으로 사용하는 지 감이 잘 오지를 않네요 🤔
인터페이스가 있다면, 추상 클래스는 존재하지 않아도 된다는 것일까요? 강결합이긴 하지만, 실제로 요구사항에서 강결합이 필요하다면 추상 클래스를 사용해야 하는 걸까요?
너무 어려운 주제같습니다 ㅎㅎ 🥵
There was a problem hiding this comment.
저 역시 이 부분이 너-무 어려운데요...ㅎㅎ
최종 코테 대비를 함께 하시던 스터디원중 한 분은 "어떤 기술을 사용하기 위한 조건을 확인하는 과정이 많이 번거롭다면 해당 기술을 배제하고 대체제를 사용하는 편이다." 라는 의견을 주셨던 것이 기억납니다!!
확실한 is-a 관계를 파악하는 것이 제일 핵심일 것 같은데요.
상속으로 구현할 수 있는 상황의 대부분은 인터페이스를 사용한 전략패턴 등으로 손쉽게 대체가 가능하기에 더더욱 상속이 적절한 상황을 찾기 어려워 지는 것 같습니다...ㅎㅎ
| import java.util.Set; | ||
|
|
||
| public class HolidayConfig { | ||
| public static final Set<MonthDay> holidays = Set.of( |
There was a problem hiding this comment.
저는 이 부분을 RosterDay 내부에서 직접 상수로 가지고 있는 것이 좋을 것 같다는 코멘트를 드리려 왔습니다.
결국 "날짜가 법정 공휴일인지 판단" 하는 책임은 날짜를 나타내는 RosterDay의 책임이고, 그렇기에 법정 공휴일의 정보 역시 RosterDay가 가지고 있는 것이 응집도가 높아진다고 생각했기 때문이에요.
다만, 그렇게 될 경우 RosterDay의 책임이 과하게 커질 수 있을 것 같기에 취향의 영역이라 생각 됩니다!
| public class RosterDay { | ||
| private static final DateTimeFormatter FORMAT_PATTERN = DateTimeFormatter.ofPattern("M월 d일"); | ||
|
|
||
| private final MonthDay monthDay; |
| } | ||
|
|
||
| @Override | ||
| public String toString() { |
There was a problem hiding this comment.
toString()을 이용해 출력 메시지를 처리해 주신 것 같아요.
이 부분에 대해선 정말 다양한 의견이 있는데, 많은 사람이 toString() 메서드는 디버깅을 위한 용도임을 주장합니다.
관련 아티클 하나 첨부 드립니다. Java 에서 toString 메소드의 올바른 사용 용도에 대하여
물론 이는 다양한 의견 중 하나일 뿐이기에 고민해 보시고 적절한 방법을 찾으시면 좋을 것 같습니다.
+) 다만, 도메인 객체가 출력의 책임을 가지게 된다면 출력 형식의 변화가 생겼을 때 일반적인 출력의 기능을 제공하는 view 계층이 아닌 도메인 로직을 수정하게 됩니다. 때문에 도메인 객체에서 출력의 책임을 분리해 내기도 하는 것으로 알고 있습니다.
There was a problem hiding this comment.
도메인 객체에서 출력을 담당하기는 하지만, 해당 객체가 다른 메서드 없이 단순히 값만을 제공하는 객체이기 때문에 toString을 통해서 표현했습니다. 실제 로직을 수행하는 Roster나 CombinedRoster와 같은 객체에는 toString이 없는 것이 그 이유예요.
해당 객체에서 get을 사용해서 뽑을 수도 있었겠지만, 그렇게 되면 또 객체에게 메시지를 던지는 느낌보다는 직접 속성을 뽑아서 출력하는 느낌이 들어서, 어떻게 보면 출력을 위한 객체라고 할 수도 있을 것 같습니다.
There was a problem hiding this comment.
아하, 일종의 DTO, 또는 VO와 비슷한 역할을 구현하시려고 했던 것 같아요!
이해가 되었습니다!!
|
전체적으로 5시간이라는게 믿기지 않을 정도로 코드 분리를 정말 잘하신 것 같아요! |
| public boolean isSameAs(Roster roster) { | ||
| int count = (int) workers.stream() | ||
| .filter(roster::contains) | ||
| .count(); | ||
|
|
||
| return count == workers.size(); | ||
| } |
There was a problem hiding this comment.
해당 부분이 제가 이해하기론 Roster과 전부 일치하는지
확인한다고 느꼈습니다! (아니라면 말해주세요!)
그렇다면 위 부분을
return workers.stream()
.allMatch(roster::contains)로 해줄 수도 있겠다는 생각이 들었어요!
There was a problem hiding this comment.
맞습니다! stream은 알아두면 알아둘수록 좋은 것 같습니다.. 다섯 줄을 한 줄로 줄일 수 있다니 😮
| private Worker getSkippedWorker() { | ||
| initIterator(); | ||
| Worker worker = iterator.next(); | ||
| moveIteratorNext(); | ||
| skipped = false; | ||
|
|
||
| return worker; |
There was a problem hiding this comment.
Iterator을 사용하면서 얻은 장점 같습니다..
전반적으로 주요 클래스가 정말 깔끔한것 같아요 5시간 안에 어떻게 하셨는지... 👍👍
There was a problem hiding this comment.
다른 곳을 다 제쳐두고 이 부분 디버깅만 한 시간 넘게 걸린 것 같아서, 리팩터링이 잘 되지 않아 아쉬움이 남는 곳이기도 해요 😅
There was a problem hiding this comment.
5시간이라는 제한이 있었으니 전 충분히 자랑하실만 하다고 봅니다...! 멋져요! 👍👍
| } | ||
|
|
||
| public RosterDay get(int day) { | ||
| return new RosterDay(MonthDay.of(month, day), startDayOfWeek.plus(day - 1L)); |
There was a problem hiding this comment.
저도 startDayOfWeek.plus 이런 부분 처럼
저는 수제 코딩(?)으로 했던 부분을 이렇게 구현하신 걸 보고 '저런 게 있었구나ㅠㅠ'하고 갑니다ㅎㅎ
There was a problem hiding this comment.
개인적으로 프리코스 때 Iterator를 지나가면서 보기만한 개념인데 그 때 공부해볼걸 그랬나 하는 아쉬움이 남을정도로 최종에서 적절하게 사용하신 게 눈에 들어왔습니다! 자극받아서 공부해보겠습니다🥹
| private static final DateTimeFormatter FORMAT_PATTERN = DateTimeFormatter.ofPattern("M월 d일"); | ||
|
|
||
| private final MonthDay monthDay; | ||
| private final DayOfWeek dayOfWeek; | ||
|
|
There was a problem hiding this comment.
DateTimeFormatter 부터 MonthDay 까지 자바 API를 많이 알고 계시네요!!
많이 배워갑니닷!
| public class Roster { | ||
| private static final int MIN_WORKERS = 5; | ||
| private static final int MAX_WORKERS = 35; | ||
|
|
||
| private final List<Worker> workers; | ||
| private ListIterator<Worker> iterator; | ||
| private boolean skipped = false; |
There was a problem hiding this comment.
이터레이터는 마지막에 도달하면 더이상 다음값을 못 가지고 와서 저는 따로 LoopIterator를 직접 만들었는데, 기존의 Iterator를 감싸는 방식이 더 좋은 것 같네요!👍👍
There was a problem hiding this comment.
시간이 남았더라면 이터레이터 래퍼 클래스를 작성했을 것 같습니다 😢
| private void moveIteratorPrevious() { | ||
| if (!iterator.hasPrevious()) { | ||
| iterator = workers.listIterator(workers.size() - 1); | ||
| return; | ||
| } | ||
| iterator.previous(); | ||
| } |
There was a problem hiding this comment.
이터레이터에 previous를 제공하는지 살면서 처음 알았네요,, 많이 배워갑니다!
There was a problem hiding this comment.
그냥 Iterator에는 없고, ListIterator에는 있더라고요! 양방향 리스트 느낌인 것 같습니다 👍🏼
(소감문)
지금까지 진행했던 프리코스 미션은 5시간이 아니라 5일간의 대장정이었습니다. 짧게는 몇 시간, 길게는 사흘까지 설계에 대해서 고민했고, 구현에 옮기고 리팩터링과 테스트를 모두 진행하느라 많은 시간을 들였습니다.
하지만 오늘 진행한 최종 코딩 테스트에서는 5시간 안에 문서 작성과 기능 구현을 비롯해 많은 것을 빠르게 해내야 했습니다. 테스트는 뒷전으로 미루게 되었고, 그 과정에서 잘 짰겠거니 생각했던 여러 곳에서 원하지 않는 동작을 발견하게 되었네요.
테스트의 중요성을 다시 깨닫습니다. 쫓기듯 달려와 어렵게 완성했지만, 정작 나의 코드에 대한 자신이 없었습니다. 마지막 한 시간동안은 디버깅과 새로운 테스트를 만드는 데 주력했습니다. 여전히 모자라 보이고, 어딘가에서 정상적으로 작동하지 않을 것만 같지만, '그럴 것 같다'를 해소해 주는 테스트가 정작 없으니 불안했습니다. 이를 빠르게 해소하고 싶었습니다.
구현의 측면에서 말하자면, Iterator를 통해서 탐색하는 아이디어를 통해서 효율적으로 탐색할 수 있게 되었습니다. 매번 선형으로 다음 근무자를 구하는 것은 꽤나 무거운 연산으로 보였기 때문에, 이를 어떻게 해결해야할 지에 대해서 시간을 많이 소모했습니다.
도메인과 서비스의 차이에 대해서도 조금 생각할 만한 문제인 것 같습니다. 현재 구현된 모습에서는
Roster가 꽤나 많은 책임을 가지고 있습니다. 어떻게 보면 로직을 수행하는 쪽에 더 치우쳐진 것 같기도 합니다. 도메인, 서비스 사이의 경계가 크지 않다고 생각하다보니 지금의 결과가 나온 듯합니다. Iterator가 차지하는 비중이 크다 보니, 이를 따로 뽑아내서 진행하면 더 좋았을 것이라고도 생각해요.다른 참가자분들이 어떻게 구현했을지가 제일 궁금합니다. 주변 사람들에게 이번 미션도 코드리뷰를 받아볼까 해요. 빠르게 작성하는 만큼 나의 코드 습관이 드러날 것 같아 걱정됩니다. 무의식 중에서 떠오르는 나쁜 습관이 코드에 묻어나지 않았을까요?
다른 사람들과 함께 코드를 짠다면 이 과정에 조금 더 자신이 붙을 것 같습니다. 5주 넘는 기간동안 달려온 나에게 수고했다고 말 해주고 싶어요. 고생 많으셨습니다 모두!