[2단계 - 출석 다시 구현하기] 웨이드(고선제) 미션 제출합니다 #160
Conversation
src/main/java/domain/SchoolDay.java
Outdated
| public static void validateSchoolDay(LocalDate date) { | ||
| if (!isPossibleAttendance(date)) { | ||
| throw new IllegalArgumentException(String.format("[ERROR] %s은(는) 등교일이 아닙니다.", customFormat(date))); | ||
| } | ||
| } | ||
|
|
||
| public static boolean isPossibleAttendance(LocalDate date) { | ||
| if (isHoliday(date)) { | ||
| return false; | ||
| } | ||
| return Arrays.stream(SchoolDay.values()) | ||
| .filter(schoolDay -> equalDayOfWeek(schoolDay, date.getDayOfWeek())) | ||
| .map(SchoolDay::isPossibleAttendance) | ||
| .findFirst() | ||
| .orElseThrow(() -> new IllegalArgumentException("[ERROR] 해당하는 요일을 찾지 못했습니다")); | ||
| } |
There was a problem hiding this comment.
public 메소드간 호출함으로써 의존하게 되는데, 이 방법이 괜찮은지 아니면 코드의 중복이더라도 따로 구현하던가, private으로 다시 생성하여 사용해야하는 지, 라라의 생각이 궁금합니다!
제가 고민했던 이유는 Public 메소드는 객체 행위를 나타낸다 생각하는데, 어쩌면 아주 작은 API?와 같은 것으로요!
그래서 public 메소드는 각각 다른 요구사항을 충족시켜주고 있다고 생각합니다
근데 서로 의존함으로써 자신의 요구사항이 아닌, 다른 요구사항의 변경으로도 전혀 상관없는 public 메소드에 영향이 갈 수도 있을 것 같아 고민했습니다!
| private static boolean isAbsence(int dayStartHour, AttendanceTime attendanceTime) { | ||
| return (attendanceTime.isEqualHour(dayStartHour) && attendanceTime.isAfterMinute(ABSENCE_LIMIT_MINUTE)) | ||
| || attendanceTime.isAfterHour(dayStartHour); | ||
| } |
There was a problem hiding this comment.
이 요구사항을 TDD로 구현하게 되고, 리팩토링하는 과정에서
attendaceTime.isEqualHour()를 구현하게 되었는데,
이처럼 리팩토링하면서 구현하게 된 간단한 메소드들은 TLD를 해야하는 것일까요? 아니면, 이것도 리팩토링하다가 새로 추가되는 메소드를 위해 TDD를 진행해야하는 걸까요?
| public void execute() { | ||
| String nickName = consoleView.requestNickName(); | ||
| LocalDate date = applicationDate.getApplicationDate(); | ||
| AttendanceTime attendanceTime = requestAttendanceTime(); | ||
|
|
||
| attendanceManager.processAttendance(nickName, date, attendanceTime); | ||
|
|
||
| Attendance attendance = attendanceManager.retrieveAttendance(nickName, date); | ||
| consoleView.printAttendanceResult(AttendanceResult.from(attendance)); | ||
| } |
There was a problem hiding this comment.
이 메소드 구현 내용으로는 출석 결과를 출력하는 부분이 마지막에 있었습니다!
제가 여기서 고민했던 점은
출석을 하기위해 주어진 데이터(nickName, date, attendanceTime)로 출석 결과를 출력하는 것이 좋은지?
아니면, 출석이 잘 되었는지 확인하기 위해 추가된 출석을 다시 조회해서 해당 출석 정보로 결과를 출력하는 것이 좋은지?
였습니다.
저는 후자를 선택했는데, 출석이 정확히 되었다는 것을 보장할 수 있었기 때문입니다.
근데, 불필요한 조회 리소스가 낭비되는 것이 아닐까 생각했습니다
processAttendance()가 잘 수행되었고, 예외가 발생하지 않았다면 믿고 전자의 방법을 선택하는 것이 괜찮을지도 모른다고 생각했었습니다!
라라의 생각이 궁금해요!
| # 얻어가고 싶은 목표 | ||
| - [x] TDD의 장점을 조금이라도 느껴보기 | ||
| - [x] 순수한 도메인 모델 | ||
| - [x] 자신 도메인으로만 구현 가능한 비즈니스 로직은 도메인 내부에 구현하기 | ||
| - [x] 여러 도메인의 조합으로 구현해야하는 비즈니스 로직은 해당 책임을 가질 객체를 생성하기 | ||
| - [x] 내가 구현한 (거의)모든 코드에 대해 근거를 말할 수 있기 |
| ## TDD를 해보면서 느낀 장점들 | ||
| 1. 내가 구현한 코드에 대해 가능한 모든 경우의 수에 대해 테스트를 작성하게 된다. 그러므로 신뢰성이 올라간다 |
There was a problem hiding this comment.
커밋 내역을 보니 대체적으로 TDD를 잘 지켜주셨네요. 👍🏻
추가로 질문에 대한 답변드립니다!
1. 비즈니스 로직의 위치
한 도메인 내에서 너무 많은 책임을 갖게 될 경우, 정책과 같이 객체를 분리하는 것도 좋은 방법이라고 생각해요. 👍🏻
저는 객체 이름에서 이 객체가 무엇을 다루는지 명확히 나타내야 한다고 생각해요!
출석 정책, 경고 정책 이라고 말씀해주신 것처럼 AttendancePolicy, CrewPenaltyPolicy 로 네이밍 의미를 명확히 해보는건 어떨까요?
2. 객체의 깊이
AttendanceRecords의 getter로 객체 그래프를 한번 끊고자 했는데 괜찮을까요??
해당 내용을 잘 이해하지못했는데, 조금 더 설명해주실 수 있을까요?
도메인 간 결합도가 생긴것일까요?
AttendanceRecords가 AttendanceTime이나 AttendanceStatus에 직접 의존하고 있다면, 결합도가 생긴 것이 맞죠!
어떤 문제점이 있으실 것 같아서 그러시나요? 🤔
일단 도메인 객체들이 상호작용해야 하는 경우가 많기 때문에 일정 수준의 결합도를 완전히 없앨 수는 없지만 최소화할 수는 있을 것 같아요.
결합도를 최소화하려면 책임을 잘 분리하고, 필요 이상으로 객체들이 서로 의존하지 않도록 설계하는 것이 중요할 것 같아요.
3. TDD 질문
요런 장단점을 느끼셨군요~!
일단 기능 요구사항 목록을 상세하게 작성하는 것이 도움이 되는 것 같아요. 각각의 기능들이 주로 하나의 메서드가 되었던 것 같거든요!
그리고 요구사항 문서에 도메인 객체들을 작성하고, 작은 단위부터 작성하며 진행하였던게 도움이 되었던 것 같아요.
예시)
Attendance : 출석 관리
- 크루별 출석 정보를 추가/수정한다.
- 출석 정보가 추가 가능한지 검증한다.
AttendanceTime : 출석 관련 시간 관리 역할
- 출석이 가능한 시간인지 검증한다.
AttendanceDate : 출석 관련 날짜 관리 역할
- 출석이 가능한 날짜인지 검증한다.
처음에는 어렵고 개발 속도를 더디게 만드는 단점이 더 큰 것 같지만, 복잡한 시스템일수록 호떡이 느끼신 장점이 극대화되어 느껴지실거에요.
그리고 반복하여 연습하다보면 시간이 오래 걸리는 부분은 점차 익숙해지실거에요! 화이팅 😀
There was a problem hiding this comment.
먼저 답변이 너무 늦어진 것 같아 죄송하다는 말 드립니다..ㅠ
정확한 네이밍을 가져야 한다는 점을 알고 있는데도 이름 짓는게 너무 어렵네요ㅠㅠ
많이 경험해보고 고민해보며 연습하겠습니다
조언 해주신대로 클래스명을 수정해봤어요!
fc70ce8
하나의 일을 해결하려는데 객체가 객체를 호출하고, 호출된 객체가 객체를 호출하는 등 캡슐화를 지키려다 보니, 객체의 깊이가 정말 깊어지게 된다는 것을 느꼈습니다.
그래서 중간에 getter를 사용해, 객체의 깊이를 끊는 것이 괜찮은지 물어보고 싶었습니다!
또, getter를 지양하라는 말이 있으니까요!
결합도를 최소화하는 방향으로 생각하겠습니다. 너무 이상적인 코드를 원한 것 같았어요.
뭐든지 극단적으로 분리하겠다. 이런 생각보다 조금더 유연하게 생각해야겠다는 생각이 듭니다
감사합니다
첫 번째로 요구사항을 정확하게 분석하고 도메인 객체를 작성한 뒤, 작은 단위의 기능을 세세하게 적어보도록 노력하겠습니다.
열심히 꾸준하게 TDD 해보겠습니다! 감사합니다!
(저는 웨이드입니다! ㅎㅎ)
| import java.time.LocalDate; | ||
| import java.time.LocalDateTime; | ||
|
|
||
| public interface ApplicationDate { |
There was a problem hiding this comment.
현재 저는 컨트롤러 테스트를 작성하지 않았기에, 해당 클래스는 필요없을 수 있을 것 같습니다.
하지만, 제 의도는 컨트롤러 테스트 작성시 기존 현재 시간을 나타내야 했던 LocalDateTime.now()를 사용하는 코드의 테스트를 작성하기 위함이었습니다.
통제할 수 없는 LocalDateTime.now()를 인터페이스로 구현하여 의존성을 분리 시킨뒤, 테스트 코드에서 인터페이스를 재구현하여 테스트에 용이할 수 있는 코드를 작성할 수 있을 것이라 생각했습니다
src/main/java/view/Menu.java
Outdated
| public static boolean isQuit(Menu menu) { | ||
| return menu == QUIT; | ||
| } |
There was a problem hiding this comment.
static 메서드가 아닌 일반 메서드로 선언할 수 있을 것 같아요.
| public static boolean isQuit(Menu menu) { | |
| return menu == QUIT; | |
| } | |
| public boolean isQuit() { | |
| return this == QUIT; | |
| } |
There was a problem hiding this comment.
아 그럴수 있네요! 필요치 않는 static 메소드를 사용했던 것 같습니다
좀더 고민해보고 사용해볼게요!
7484ecf
| private static final int OPERATING_OPEN_HOUR = 8; | ||
| private static final int OPERATING_CLOSE_HOUR = 23; | ||
|
|
||
| public AttendanceTime(int hour, int minute) { |
There was a problem hiding this comment.
아 넵 알겠습니다!
record에 필요치 않는 코드였네요! 수정하였습니다!
| if (this == o) { | ||
| return true; | ||
| } | ||
| if (!(o instanceof AttendanceTime that)) { |
There was a problem hiding this comment.
왜 저에겐 뜨지 않는 걸까요..ㅠ 한번 찾아보겠습니다!
| public class PenaltyTargetComparator implements Comparator<PenaltyTarget> { | ||
|
|
||
| @Override | ||
| public int compare(PenaltyTarget o1, PenaltyTarget o2) { |
There was a problem hiding this comment.
o1, o2 대신 조금 더 구체적인 네이밍을 사용해볼까요? 🙂
There was a problem hiding this comment.
오버라이딩 메소드이기 때문에, 구체적인 이름을 짓지 않았는데
오버라이딩한 메소드도 구체적으로 모두 매개변수 혹은 변수명을 지어줘야 하는 걸까요?
equals & hascode, compare, 등등!
라라의 의견이 궁금해요
| } | ||
|
|
||
| private ConsoleView consoleView() { | ||
| return ConsoleView.create(new InputView(), new OutputView(), new InputParser(), new OutputParser()); |
There was a problem hiding this comment.
호출될 떄마다 매번 새로운 인스턴스들을 생성할 필요가 있을까요~?
There was a problem hiding this comment.
저도 이부분을 고려해서 ConsoleView의 팩토리 메소드 내에 인스턴스가 하나만 구현되도록 구현하였습니다!
|
리뷰 해주시느라 고생하셨습니다! 라라! |
체크 리스트
test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?프롤로그 링크(수정본)
객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?
1~5점 중에서 선택해주세요.
선택한 점수의 이유를 적어주세요.
제가 작성한 코드를 완벽하다고 말할 수는 없고... 객체지향 생활체조 요구사항을 거의 잘 지켰다고 생각해서 4번을 골랐습니다!
어떤 부분에 집중하여 리뷰해야 할까요?
안녕하세요 라라! 이번에도 잘 부탁드립니다!!
1. 비즈니스 로직의 위치
지난 step1 때 모든 비즈니스 로직의 위치가 도메인 내부에 있어, 여러 도메인이 서로 결합되어 결합도가 높아지는 것 같았습니다
그래서 이를 해결하기 위해 자신의 도메인으로 해결할 수 있는 것은 최대한 자신의 도메인 내부에 비즈니스 로직을 구현하고,
여러 도메인이 섞인다면 섞인 도메인만을 위한 객체를 만들어 미션을 구현했습니다.
예를 들어,
AttendanceManager = 출석(Attendance) + 출석 정책(SchoolDay)
CrewPenaltyManager = 출석(Attendance) + 경고 정책(Penalty)
애매하게 나눈 것 같아서 제 3자 입장에서 바라봐주시면 감사하겠습니다.
근데 사용 결과 AttendanceManager는 단지 CrewAttendanceBook이라는 객체를 상태로 가지고 역할을 수행하는 또다른 도메인 객체 느낌이고,
CrewPenaltyManager는 서비스 느낌이 좀 있는 것 같아서..ㅠ 클래스명을 바꿔야할 것 같은지도 드시는 생각 얘기해주시면 감사하겠습니다!
2. 객체의 깊이
현재 객체의 깊이가 아래처럼 너무 깊어져서 고민이 있었습니다.
그래서 AttendanceRecords의 getter로 객체 그래프를 한번 끊고자 했는데 괜찮을까요??
그리고 또, 궁금한 점은 위에서 만약 AttendanceRecords가 AttendanceTime에 직접 의존하고 있다면, 혹은 AttendanceStatus에 의존하고 있다면 도메인 간 결합도가 생긴것일까요?
조금 애매모호하게 질문드린 것 같아서 죄송합니다! 그냥 글 보시고 생각나는 것 피드백해주시면 감사할것 같아요!
3. TDD 질문
TDD를 열심히 해봄으로써 장점은 확실히 느껴졌습니다! 신뢰도 높은 코드를 작성할 수 있다는 점을 알게된 것 같습니다!
TDD에서 가장 어려웠던 점은 요구사항을 정확히 인지하고 어떤 단위로 테스트를 작성할 것인지 생각하는 것이 너무 어려웠던 것 같아요..!
단점도 확실하게 느꼈는데, 시간이 정말 오래걸리는 것 같아요ㅠ.. 아직 익숙하지 않아서 그런걸까요?
궁금한 점은 TDD를 하게 되면서 불필요한 메소드를 자꾸 정의하게 되고 생성하게 되어서, 나중에는 안쓰는 것을 삭제하려고 찾아다녔던 것 같습니다.. 이걸 방지하기 위한 라라만의 팁이 있을까요??