[2단계 - 출석 다시 구현하기] 아이나(육새라) 미션 제출합니다. #166
Conversation
| private void validateName(final String name) { | ||
| if (!crewAttendances.containsKey(name)) { | ||
| throw new IllegalArgumentException("[ERROR] 유효하지 않은 닉네임입니다."); | ||
| public void validateNickname(final String nickname) { |
There was a problem hiding this comment.
validate 대신에 조금 구체적인 네이밍을 사용해주셔도 좋을 것 같네요.
There was a problem hiding this comment.
validateName()은 등록된 크루의 닉네임이 맞는지 검증한다는 의미를 잘 전달하지 못하는 것 같아 throwIfNotRegisteredCrew()로 변경했습니다~!
| return count; | ||
| } | ||
|
|
||
| private int updateAbsentCount(final LocalDateTime targetDay, final int count) { |
There was a problem hiding this comment.
여기서 boolean 반환하는 식으로 시그니처를 변경하는게 조금 더 자연스러울 수 있을 것 같네요.
There was a problem hiding this comment.
indent를 줄이기 위해 메서드를 분리하면서 count 값의 증가가 분리된 메서드에서 이루어지게 됐는데, 메서드를 분리하지 않아도 indent가 깊어지지 않는 방법이 생각나 countAbsent() 내부 로직을 수정했습니다!
|
|
||
| private final String displayName; | ||
| private final int absentCountUnderBound; | ||
| private Map<Crew, CrewAttendance> crews; |
There was a problem hiding this comment.
enum은 싱글턴이기 때문에 어플리케이션 전체에서 동시에 접근할 수 있습니다.
싱글턴 패턴 내에 상태를 저장하면 어떤 일이 발생할까요?
이 질문을 조금 더 포괄적으로 변경해보면 static 키워드를 붙인 변수를 어플리케이션 로직 내에서 변경해도 될까?가 될 것 같네요. 한 번 고민해보시면 좋을 것 같아요.
| } | ||
|
|
||
| public String getDisplayName() { | ||
| return displayName; | ||
| } | ||
|
|
||
| public Map<Crew, CrewAttendance> getCrews() { |
There was a problem hiding this comment.
일단 구현을 마치기 위해 getter를 사용하거나 더 효율적인 로직으로 리팩터링 하지 못해 필요한 getter와 불필요한 getter가 혼재할 것 같습니다. 개선이 가능한 부분은 짚어주시면 도움이 될 것 같습니다 :)
로직 전반적으로 메세지 던져주시면서 잘 구현해주신 것 같습니다 👍
또한 불변 객체를 사용하다보니 getter의 사용이 늘어났는데 괜찮은 수준으로 사용한 것인지, 혹은 불변 객체를 잘못 사용하고 있는 것인지 궁금합니다!
당장 크게 눈에 띄는 부분은 없는 것 같네요.
| private static void printCrewsOf(final WarningLevel warningLevel, final LocalDateTime today) { | ||
| final Map<Crew, CrewAttendance> crews = warningLevel.getCrews(); | ||
| for (Entry<Crew, CrewAttendance> entry : crews.entrySet()) { | ||
| Crew crew = entry.getKey(); | ||
| CrewAttendance crewAttendance = entry.getValue(); | ||
| final Map<AttendanceStatus, Integer> attendanceStatusCounts = | ||
| crewAttendance.countAttendanceStatusesBefore(today); | ||
| System.out.print(formatWarningCrew(warningLevel, crew, attendanceStatusCounts)); |
There was a problem hiding this comment.
이 로직도 enum의 상태 필드에 의존하고 있네요.
결과를 컨트롤러에서 만들어서 view로 넘겨주는 방식으로 수정해주세요.
✅ 체크 리스트
test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?🔢 객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?
1~5점 중에서 선택해주세요.
선택한 점수의 이유를 적어주세요.
1단계 구현 시 보다는 원시값과 문자열을 최대한 포장하고, 엔티티를 작게 유지했다는 점에서 1점이 올랐다고 생각합니다.
그러나 불변 포장 객체를 사용하면서 getter의 사용이 늘어난 것 같아 이 부분에 대한 리팩터링이 필요합니다.
또한 getter 사용으로 디미터 법칙을 지키지 못한 부분이 존재할 수도 있을 것 같습니다.
🧐 어떤 부분에 집중하여 리뷰해야 할까요?
범블비 안녕하세요! 아이나입니다 :)
출석 미션 2단계 구현은 TDD에 익숙해지는 것과 1단계 구현 시 아쉬웠던 부분을 개선하는 것을 목표로 진행했습니다.
두 가지 목표에 대한 회고와 궁금한 점을 정리해보았습니다.
TDD
도메인에 대한 이해가 높아진 상태에서 다시 TDD를 적용하니 이전보다 테스트 코드 작성이 수월했습니다.
하지만 반대로 도메인을 파악하고 있다 보니 테스트의 단위가 커졌다는 단점이 있는 것 같습니다.
❓도메인을 이해할수록 테스트 단위가 커지는 것은 당연한 현상일까요? 혹은 그렇다 하더라도 의식적으로 테스트 단위를 작게 쪼개는 연습을 하는 것이 좋을까요?
❓이번에도 커밋을 Red, Green, Refactor 세 단계로 나누어 진행했는데 혹시 Red 단계에서 테스트 코드가 적절하게 작성되지 못했거나 더 개선할 수 있을만한 부분이 있다면 알려주시면 감사하겠습니다 :)
코드 개선
미션 제출을 끝내고 보니 1단계 구현에 걸린 시간과 2단계 재구현에 걸린 시간이 거의 비슷했습니다.
비슷한 시간 동안 구현한 두 버전을 비교하니 미처 구현하지 못하고 남은 기능 또한 비슷한 수준이었습니다.(제적 위험자 정렬 기능과 캠퍼스 운영 시간 적용 등)
달라진 부분은 코드가 조금 더 정돈되었고, 테스트 케이스가 늘어났다는 것입니다.
1단계에서 주신 피드백과 추가로 학습한 내용을 적용해 불변 객체를 사용함으로써 객체를 더 작게 유지할 수 있었습니다.
그리고 테스트 코드 작성에 더 많은 시간을 투자해 다양한 테스트 케이스를 검증하고 코드를 정돈할 수 있었습니다.
개선된 부분도 있지만 프로그램의 각 기능이 실행되는 로직은 이전과 거의 비슷하게 구현했다는 점이 아쉽습니다.
기능 실행에 주요 역할을 하는 객체들 간의 관계에 대해 좀 더 깊게 고민해보지 못했습니다.
특히 제적 위험자 정렬을 효율적으로 할 수 있는 방법과 그 책임에 대해 발전된 부분이 없어 더 고민해보려 합니다.
❓이와 관련해서 부족한 부분들에 대해 피드백 주시면 감사하겠습니다!
❓또한 불변 객체를 사용하다보니 getter의 사용이 늘어났는데 괜찮은 수준으로 사용한 것인지, 혹은 불변 객체를 잘못 사용하고 있는 것인지 궁금합니다!
❓일단 구현을 마치기 위해 getter를 사용하거나 더 효율적인 로직으로 리팩터링 하지 못해 필요한 getter와 불필요한 getter가 혼재할 것 같습니다. 개선이 가능한 부분은 짚어주시면 도움이 될 것 같습니다 :)
그 외 아쉬운 점