Step2 리뷰요청 드립니다. #819
Step2 리뷰요청 드립니다. #819
Conversation
…) -> Money, Capacity 클래스 구현, Payment 클래스 수정
(Session 구현 전, 빠진 부분 점검..)
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
질문에 답변만 하기 위해 코드 리뷰는 진행하지 않았어요.
코드 리뷰는 요구사항을 완성한 후에 진행하도록 할께요.
Q1. PaidEnrollmentPolicy에서는 Capacity에 대한 내용을 추가해서 Money와 Capacity에 대한 클래스를 구현했는데,
Session에서 수강신청(enroll으로 만들 예정입니다)을 하면, 현재 수강생을 증가하는 메소드가 필요하지 않을까 하는 부분
이런 빠진거 같은 부분은 Session 단계에서 알게되었으면 그때 다시 돌아가서 추가하면 되는 과정일까요?
A1. 맞습니다. 처음부터 요구사항 분석을 완벽히 한 후 객체 설계를 진행할 수 있는데요. 우리가 처음 요구사항을 받았을 때 모든 요구사항을 빠트리지 않고 분석하기란 쉽지 않을 수 있습니다.
도메인 지식은 프로그램을 구현하다보면 높아지기 때문에 높아지는 도메인 지식에 따라 객체 설계가 바뀌거나, 빠트린 요구사항을 추가하는 것이 당연한 접근 방식입니다.
프로젝트 진행 중 요구사항이 변경되는 경우도 다반사이기 때문에 초반에 완벽한 설계를 하는데 집중하기 보다 지속적인 리팩터링 역량이 훨씬 더 중요하다 생각합니다.
미션 마무리한 후 다시 리뷰 요청 주시면 코드 리뷰 진행할께요.
|
일부 로직 수정 후 이어서 진행해보았습니다. 실무에서는 요구사항을 받으면 어떤 DB에 들어가야하지? 를 가장 먼저 고민하게 되고, 그 부분을 어드민 화면에서 먼저 작업하고, 이런 미션이 좋은 경험을 하는거 같아서 재밌게 하다가도 그런 부분이 쉽진 않은거 같습니다 피드백 주시면 전체적으로 다시 한번 생각해보겠습니다! |
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
전체적인 객체 설계, 단위 테스트 코드 구현 💯
확실히 이전 미션 진행할 때는 여유가 없어서 그랬는지 아쉬운 부분이 가끔 보였는데요.
지금은 시간을 가지면서 미션을 진행해서인지 전체적인 객체 설계와 접근이 좋네요.
이전 미션에 비해 복잡도가 낮은 것은 아닌데 한 단계 성장했음을 느낄 수 있네요.
단, pr 본문에도 말한 것처럼 일부 접근을 db 관점에서 접근하고 있네요.
예를 들어 session id가 필요 없음에도 불구하고 id 추가, 수강생 목록을 List와 같은 콜렉션이 아닌 역정규화한 현재 수강생 수로 관리하는 부분등이 보이네요.
db에 대한 생각을 버리고 도메인 객체 설계에 집중해 보면 어떨까요?
db에 대한 고려는 3단계에서 해도 좋을 것 같아요.
Q1. 이미 설계되어진 구조에서 이런 연습을 하려면 어떻게 접근해보는게 좋을까요?
A1. 현재 연습은 기존에 없던 새로운 기능을 추가하거나, 프로젝트를 새로 시작하는 시점에 적합한 접근 방식이고요. 이미 구현되어 끝난 레거시 코드는 1단계와 같이 점진적으로 도메인 객체를 분리해 나가는 방식으로 접근할 수 밖에 없습니다. 구현은 되어 있지 않은데 설계만 되어 있다면 db 테이블에 의존하지 않으면서 도메인 객체를 설계하는 역량을 키울 수 밖에 없을 것 같아요. 현재와 같이 db 테이블 없이 도메인 객체를 먼저 구현하는 연습이 도메인 객체 설계 역량을 키울 수 있는 좋은 접근 방식이라 생각해요.
|
|
||
| import java.time.LocalDateTime; | ||
|
|
||
| public class Enrollment { |
|
|
||
| import nextstep.payments.domain.Payment; | ||
|
|
||
| public interface EnrollmentPolicy { |
| this.sessionState = sessionState; | ||
| } | ||
|
|
||
| public Enrollment enroll(Long userId, Payment payment) { |
There was a problem hiding this comment.
👍
단, 수강 신청이 가능한 경우 수강생 목록에 추가해야하지 않을까?
즉, Session이 수강생 목록을 가지고 있어야 하지 않을까?
수강생 목록에 수강생을 추가할 때 이미 수강 신청한 수강생인지, 수강생 최대 인원을 초과하는 등의 판단을 해야하지 않을까?
| this(id, new SessionDuration(startDate, endDate), new CoverImage(size, fileName, width, height) | ||
| , enrollmentPolicy, sessionState); |
| Session session = new Session(sessionId, LocalDateTime.now(), LocalDateTime.now().plusDays(7) | ||
| , 500_000, "test.jpg", 300, 200, policy, sessionState); |
There was a problem hiding this comment.
Session 객체와 같이 인스턴스 변수가 많은 객체를 테스트하려면 객체를 생성하는데 어려움이 있다.
중복 코드 또한 많이 발생해 Session을 생성할 때 생성자의 인자가 변경되는 경우 변경할 부분이 많아진다.
https://www.arhohuttunen.com/test-data-builders/ 문서 참고해 Session과 같이 복잡한 객체의 테스트 데이터를 생성할 때 어떤 방법을 사용할 것인지 선택해 보면 좋겠다.
이번 기회에 내가 선호하는 방법을 적용해 보고 앞으로도 쭈욱 활용하는 방식이면 좋겠다.
2 participants
안녕하세요!
두번째 단계를 진행하고 있습니다.
아직 완성은 되지 않았는데, 중간에 궁금한것들이 있어서 먼저 리뷰요청 드립니다.
수강 신청 기능 요구사항을 보고 밑에서 부터 조금씩 위로 올라가며 구현하고 있었습니다.
CourSe -> Session -> Date/CoverImage/Policy/state으로 보고
CoverImage는 또 ImageType, ImageSize, ImageDimension으로 나누어지고 그 안에서부터 구현을 시작하였습니다.
요구사항에 맞춰서만 구현을 해보다가 조금씩 내용이 빠진거 같은 느낌이 들었습니다.
이걸 Session을 만드려고 보니 ImageType은 타입을 직접 입력하던가...?! 보통은 이미지 이름.타입 이렇게 등록을 하지 않을까 싶었던 부분
PaidEnrollmentPolicy에서는 Capacity에 대한 내용을 추가해서 Money와 Capacity에 대한 클래스를 구현했는데,
Session에서 수강신청(enroll으로 만들 예정입니다)을 하면, 현재 수강생을 증가하는 메소드가 필요하지 않을까 하는 부분
이런 빠진거 같은 부분은 Session 단계에서 알게되었으면 그때 다시 돌아가서 추가하면 되는 과정일까요?
해당 클래스들을 구현할때, 요구사항을 텍스트 그대로만 생각하고 개발하다보니 저런 부분들을 놓친거 같습니다.
이외에도 피드백 주시면 다시 한번 생각해보고 이어서 개발 진행하겠습니다.
( 이 후 과정은 'Session 구현 -> Sessions(일급컬렉션) 구현 -> Course 완성' 으로 생각하고 있습니다! )
감사합니다!