-
Notifications
You must be signed in to change notification settings - Fork 81
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
[2단계 - 출석 다시 구현하기] 미소(이소은) 미션 제출합니다. #127
base: soeun2537
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요. 미소!
몇 가지 코멘트 남겼습니다!
함께 고민해 보면 좋을 것 같아요!
테스트 없이는 코드를 작성하지 않기 때문에 코드의 동작이 항상 검증되고 테스트 자체가 문서가 될 수 있다고 생각해요. :) 또한, 테스트가 존재하기 때문에 리팩터링이 무섭지 않습니다! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요! 미소!
가벼운 코멘트 남겼습니다!
확인 부탁드려요!
체크 리스트
test
를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?
1~5점 중에서 선택해주세요.
선택한 점수의 이유를 적어주세요.
Getter를 사용한 부분이 있어 아쉬웠기 때문입니다.
어떤 부분에 집중하여 리뷰해야 할까요?
Attendances
객체에 입력받은 데이터를 넘겨주고, 해당 객체 내부에서 실제 객체(Attendance
,Crew
)를 생성하는 방식으로 구현했는데, 이 방법이 적절한지 궁금합니다.Attendance
와Crew
객체를 직접 생성한 후Attendances
에 전달하는 방식Attendances
내부에서 객체를 생성하는 방식checkInTime
)가 비어 있는 객체를 생성하도록 했습니다. 데이터베이스 관점에서 조회를 고려하면 Step2의 방식이 더 적절하다고 생각하는데, 이에 대해 어떻게 생각하시나요?Attendances
에서Map<Crew, List<Attendance>>
형태로 데이터를 관리하고 있는데, 이 데이터 구조가 적절한지 고민하고 있습니다. 혹시 더 나은 구조나 개선할 점이 있을까요?DateTimeGenerator
를 활용하여 여러 곳에서LocalDateTime.now()
를 직접 호출하는 대신 중앙에서 관리하도록 설정했는데, 이 방식에서 발생할 수 있는 또 다른 문제나 개선할 점이 있을까요?