[2단계 - 출석 다시 구현하기] 노랑(노주희) 미션 제출합니다. #167
Conversation
- `6. 출결 관리 파일 입출력` 추가
- 최소한의 조건을 검증하는 테스트 작성 - 정말 이정도만 작성해도 될까?
- 최소한의 조건을 통과하는 테스트 작성
- 테스트 클래스 이름, 테스트 메서드 이름 수정 - 10시 5분 1초일 경우 출석이 아닌 것으로 표시되도록 수정
- 지각과 결석을 반환하는 테스트 추가
- 지각과 결석을 반환하는 최소한의 기능 추가
- 테스트에 중첩 클래스 사용 - 테스트 메서드 이름 변경 - 출결 기준 시간 상수화
- 월요일에 대한 테스트코드 추가
- 테스트를 통과하는 최소한의 코드 작성
- 출석할 날짜와 시간을 LocalDateTime 인자 하나로 받도록 수정 - 교육 시작 시간 상수화 - 지각, 결석에 해당하는 시간 차이 상수화
- 최소한의 테스트 작성
- 테스트를 통과하는 최소한의 코드 작성
- 테스트 메서드 변경 - 문자열을 포장하는 Crew 객체 생성
- 최소한의 테스트 작성
- 최소한의 코드 작성 - 공휴일도 일부만 적용
- 테스트 추가 - 테스트 메서드명 구체화 - Vacation, LegalHoliday Enum을 생성해서 상수값 관리 책임 부여
- 최소한의 테스트 작성
- 테스트를 통과시키는 최소한의 코드 작성
- ParameterizedTest 활용 - 테스트 메서드명 구체화 - 캠퍼스 운영 시간 상수화
- 최소한의 테스트 작성
- 테스트를 통과하는 최소한의 코드 작성 - 테스트코드에서 AttendanceHistories를 로컬 변수에서 멤버 변수로 변경
- 테스트 메서드명 수정 - 로직 가독성 향상을 위한 리팩터링
- 실패하는 테스트 추가 작성
- 테스트 통과하도록 로직 수정
- 잘못 작성된 코드 삭제 - 메직 넘버 상수화
- 테스트를 위한 테스트픽스처 생성 - 테스트 작성
- 테스트를 통과시키는 코드 작성
- 테스트 메서드명 수정
- 5.2 실행할 메뉴를 입력할 수 있다. - 5.5 존재하지 않는 메뉴를 선택한 경우 예외를 발생시킬 수 있다.
- 객체 생성 책임을 컨트롤러에서 도메인으로 분리
- 7.2 등교 시간을 입력할 수 있다. - 7.4 시간 입력 형식이 잘못된 경우 예외를 발생시킬 수 있다.
- 8.1 출석을 수정하고자 하는 크루의 닉네임을 입력할 수 있다. - 8.2 출석 수정할 날짜(월)을 입력할 수 있다. - 8.3 출석 수정할 날짜(일)을 입력할 수 있다. - 8.6 날짜 입력 형식이 잘못된 경우 예외를 발생시킬 수 있다.
- 8.4 등교 시간을 입력할 수 있다. - 8.7 시간 입력 형식이 잘못된 경우 예외를 발생시킬 수 있다.
- 캠퍼스 운영 요일을 계산하기 위한 Campus 클래스 추가
There was a problem hiding this comment.
이 객체가 참 모호하게 느껴져서 엘리의 의견을 구하고 싶습니다
대략적으로 이 객체는
- 우테코의 출석 시작일은 상수로 갖고 있습니다.
countValidDays()는 결석일을 카운트할 때 사용됩니다. 결석 데이터가 모두 저장되어 있는 것이 아니기 때문에 우테코 시작일로부터 인자로 받은 날짜가지 유효한 날짜를 모두 계산한 후, 거기서 출석일과 지각일을 빼서 결석일을 구할 수 있습니다.isOpen()은 캠퍼스가 여는 날인지 판단할 때 사용합니다. 이 메서드는 맨 처음에 출석 확인 기능을 구현할 때AttendanceHistories에 작성을 했는데 아무리 생각해도 이 위치는 아닌 것 같아서 끝까지 고민하다가 마지막에Campus클래스로 분리했습니다.getOpenDaysUntil()은OutputView에서 출석 기록을 출력할 때 사용됩니다. 결석 기록을 따로 저장하지 않기 때문에getOpenDaysUtil()을 이용해서 유효한 날짜들의 리스트를 먼저 받아온 후, 해당 날짜에 기록이 없으면 결석으로 출력하기 위해 만들었습니다.
이렇게 static이 붙은 객체들을 보통 util로 분류를 하던데 이 객체도 util 계층으로 분리를 하는게 맞을까요?
There was a problem hiding this comment.
Campus 객체에 대해 고민중이시군요~ 👍
우선 AttendanceHistories 는 적절한 위치가 아니라고 판단하신 이유가 무엇인가요?
개인적으로는 Campus 는 CampusHour 와 역할이 비슷해 보입니다. 날짜 / 시간 여부만 다를 뿐, 두 클래스 모두 캠퍼스 운영과 관련된 상태 및 행위를 가집니다. 두 클래스를 합쳐보는 건 어떨까요?
그리고 Campus 는 모호하다고 느끼셨지만, CampusHour 는 그렇게 느끼지 않았던 이유에 대해서도 고민해보면 좋을 것 같습니다 🙂
정답이 있는 문제는 아니니 자유롭게 고민해주세요!
+) 보통 util 클래스는 비즈니스 로직과 전혀 무관한 메서드들을 두는 곳입니다. java.util 패키지 하위의 클래스들을 참고해보세요.
따라서 Campus 를 util 클래스로 분류하긴 어려울 것 같네요!
There was a problem hiding this comment.
어쩌다보니 테스트 클래스를 하나만 작성하게 됐습니다. 약간의 변명을 해보자면..ㅎㅎ
Outside-in 방식으로 TDD를 진행하다 보니 요구사항을 바탕으로 테스트를 작성하게 되고, 결국 모든 구현의 시작은 모든 데이터가 있는 AttendanceHistories가 됐습니다. TDD 사이클을 몇번 돌린 후, refactor 단계가 되어서야 새로운 클래스들을 만들어졌ㅅ브니다.
그렇다보니 하위의 클래스들을 작성했을 때는 이미 그 클래스의 메서드들을 AttendanceHistories에서 호출하고 있고, 그에 대한 테스트도 모두 작성된 상태여서 추가 테스트 작성이 필요하다고 느껴지지 않았습니다. 또한 하위 테스트 객체의 구조가 설계에 기반한 구조가 아니기 때문에 추후 변동 될 가능성이 크다고 느껴져서 일단 테스트를 작성하지 않은 것도 있습니다.
There was a problem hiding this comment.
아하~ 어떤 과정을 거쳐 1개의 테스트 클래스로 마무리되었는지 이해했습니다 ㅎㅎ
우선 AttendanceHistoriesTest 테스트 내용에 대해서는 칭찬해드리고 싶네요 👏 💯
요구사항이 꼼꼼히 반영되었고 예외상황, 경계값 테스트도 잘 하셨어요!
노랑이 작성한 AttendanceHistoriesTest 는 단위 테스트보단 통합 테스트에 더 가까운데요!
(물론 서비스 계층이 없어 정확하게 통합 테스트라고 하긴 애매하지만, 여러 객체가 협력해 하나의 요구사항이 잘 수행되는지 테스트하는 부분은 통합 테스트와 일맥상통합니다🙂)
단위 테스트 / 통합 테스트 모두 코드 유지보수 관점에서 꼭 필요한 테스트인데요!
중복으로 느끼실 수도 있지만, 각각은 목적과 장단점이 다르기 때문에 모두 작성하는 것이 권장됩니다.
그 중에서도 빠르고 단순한 단위 테스트를 더 많이 작성하는 방식이 권장됩니다 ㅎㅎ (테스트 피라미드 라고 합니다)
refactor 단계에서 새로운 클래스들이 만들어졌다면, 그에 맞게 테스트 메서드들도 새로운 테스트 클래스로 옮겨졌다면 좋을 것 같네요. 해당 내용은 지금 단계에서 반영할 필요는 없고, 다음 미션부터 유의해주세요!
| import org.junit.jupiter.params.provider.CsvSource; | ||
| import org.junit.jupiter.params.provider.ValueSource; | ||
|
|
||
| public class AttendanceHistoriesTest { |
There was a problem hiding this comment.
JUnit5 에서는 테스트 클래스 / 메서드에 public 을 붙일 필요가 없습니다~
| public class AttendanceHistoriesTest { | |
| class AttendanceHistoriesTest { |
| public enum DisciplinaryStatus { | ||
| NONE("해당 사항 없음", 1), | ||
| WARNING("경고", 2), | ||
| ONE_ON_ONE("면담", 3), | ||
| EXPELLED("제적", 6); | ||
| NONE("해당 사항 없음", 0), | ||
| WARNING("경고", 6), | ||
| ONE_ON_ONE("면담", 9), | ||
| EXPELLED("제적", 15); | ||
|
|
||
| private static final int ABSENCE_WEIGHT = 3; |
There was a problem hiding this comment.
2, 3, 5 를 각각 6, 9, 15 로 표현하셨네요~
만약 지각 3회 = 결석 1회 라는 요구사항이 지각 4회 = 결석 1회 로 변경된다면, DisciplinaryStatus 내부 값까지 수정이 필요해지는데요..!
이건 SRP 원칙 위반일 뿐 아니라, thresholdCount 네이밍만 보고 누군가가 요구사항을 잘못 이해할 수도 있습니다.
그리고 ABSENCE_WEIGHT 는 의미상 AttendanceStatus 클래스와 더 밀접해보이네요!
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) { | ||
| return true; | ||
| } | ||
| if (o == null || getClass() != o.getClass()) { | ||
| return false; | ||
| } | ||
|
|
||
| AttendanceDateTimes that = (AttendanceDateTimes) o; | ||
| return attendanceDateTimes.equals(that.attendanceDateTimes); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return attendanceDateTimes.hashCode(); | ||
| } |
| import java.time.DayOfWeek; | ||
| import java.time.LocalDate; | ||
|
|
||
| public class AttendanceDateTimeFixture { |
There was a problem hiding this comment.
fixture 클래스는 보통 여러 테스트 클래스에서 재사용하기 위해 만들어지는데요~
때문에 date와 같은 상태를 가지지 않는 것이 일반적입니다.
또한 테스트 클래스에서는 로직 작성을 지양하는데, 이는 fixture 도 마찬가지입니다.
fixture에서 로직 및 상태를 제거하고, 사용하기 용이한 방향으로 리팩토링 해볼까요?
체크 리스트
test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?
1~5점 중에서 선택해주세요.
선택한 점수의 이유를 적어주세요.
책임 분리를 좀 부족한 것 같다
어떤 부분에 집중하여 리뷰해야 할까요?
요구사항 변경
기존 요구사항은 2024년 12월 사용을 가정으로 작성되었는데, step2를 구현하면서 2025년 7기 사용을 가정하여 구현했습니다!
LocalDate.now()를 사용해서 오늘의 날짜를 사용하도록 했습니다.기록이 없는 레코드는 저장하지 않음
기존에는 CSV 파일을 읽어온 후, 기록이 없는 날짜는 결석 객체를 만들어서 저장했는데 step1 때 리뷰를 반영하여 별도의 데이터를 저장하지 않는 방식으로 변경했습니다.
이에 따라 출석 기록을 출력하는 로직과, 결석 횟수를 구하는 로직이 크게 변경되었습니다.
집중하여 구현한 부분
크게 3가지 부분에 집중해서 구현했습니다.
질문이라기보단 저의 생각을 정리한 글입니다ㅎㅎ
TDD의 방향성
이전에 DM으로 엘리와 논의한 것처럼 step1과는 다른 방식으로 TDD를 진행해봤습니다.
찾아보니까 step1에서 사용한 방식은
Inside-out, step2에서 사용한 방식은Outside-in방식에 가깝다고 생각할 수 있는 것 같습니다. 두 가지 방식을 다 해보고 느낀점은, 우테코 미션 정도의 규모에서는Inside-out방식이 좀 더 적합하다고 느꼈습니다.Outside-in방식과Inside-out을 비교했을 때 느낀 장점은.. 필요 없는 객체를 덜 생성한다 정도였습니다.Outside-in방식과Inside-out을 비교했을 때 느낀 단점은 객체의 책임 분리가 어렵다는 것입니다.Inside-out방식으로 할 때는 작은 객체부터 bottom-up 방식으로 구현을 했는데Outside-in방식으로 할 때는 자연스럽게 최상위의 객체부터 구현을 하게 됐습니다. 초반 커밋을 보면 모든 API들이 한 객체에 모여있습니다. 아직 경험이 부족해서인지 이 뭉쳐진 책임들을 구현 과정에서 적절히 나누는 것이 어려웠습니다. 이러한 어려움과 더불어, 초반부터 설계 없이 큰 그림을 계속 생각해야 해서 오히려 코드 작성 속도가 더뎌젔습니다.만약 프로젝트의 규모가 굉장히 커서 설계 하기가 어렵고, 새로운 기능을 추가하는 상황이라면
Outside-in방식의 장점을 더 느낄 수 있을 것 같습니다. 다만 미션에 적용하기는Inside-out이 더 적절할 것 같아서 우선 Level1 미션들은Inside-out의 방식대로 진행해야겠다고 결론을 내렸습니다.MVC 구조에서 책임 분리
이번 미션을 진행하면서 요구사항에 따라 테스트를 작성하는 방식으로 진행하기 위해
README.md에 요구사항을 작성할 때 부터 도메인 요구사항과 입출력 요구사항으로 나눠서 작성했습니다. 그렇다보니 어떤 기능이 주어졌을 때, step1 때 보다 domain, view, controller 중 누구의 책임인지에 대해서 한 번 더 고민해 볼 수 있었습니다.그 중 몇가지는 모호하다고 느껴졌던 기능들이 있어서 관련 내용을 정리해봤습니다.
README.md에는 입출력 요구사항으로 분류했습니다.FileInputView를 추가해 해당 객체가 기본적인 파일 가공을 하도록 처리했습니다.Map<String, List<LocalDateTime>>자료구조로 반환하는 것 까지는FileInputView에서 처리했고, 이걸 받아서AttendanceHistories를 생성하는 것은 처음에 컨트롤러에서 했다가, 컨트롤러의 책임이 너무 무거워졌다고 판단을 해서 도메인에AttendanceHistoryGenerator를 추가해 분리했습니다. 835418c테스트 픽스처
저번 리뷰 때 언급해주신 “테스트픽스처”를 잘 적용해보고 싶어서 고민해보고 적용해봤습니다.
AttendanceDateTimeFixture는 출석 기록 하나를 만들 수 있는 객체입니다. 생성할 때 시작 날짜를 지정해줄 수 있고 원하는 출석 상태의 출석 기록을 만들어줄 수 있습니다. 유효한 출석 날짜를 연속해서 생성을 할 수 있도록 출석 기록을 만들 때마다 멤버 변수인date를 다음 유효한 출석 날짜로 바꾸도록 설계했습니다.AttendanceDateTimeFixture를 이용해서AttendanceDateTimesFixture와AttendanceHistoriesFixture에서는 특정 상태의 출석 기록을 원하는 개수 만큼 동적으로 가지는 객체를 생성할 수 있도록 설계했습니다.