-
Notifications
You must be signed in to change notification settings - Fork 309
Step3 - 수강신청(DB 적용) #823
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
base: ho-jun97
Are you sure you want to change the base?
Step3 - 수강신청(DB 적용) #823
Conversation
javajigi
left a comment
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.
db 매핑 작업하느라 수고했어요. 👍
도메인 객체 설계 후에 db 매핑 작업하면서 어떤 점을 느끼셨나요?
repository를 추가하기는 했는데 repository와 도메인 객체를 연결하는 Service 클래스를 추가하다보니 부족한 부분이 없는지 파악하는데 어려움이 있었을 것 같아요.
수강신청을 담당하는 Service 클래스 추가해 repository와 도메인 객체를 연결해보면 좋겠습니다.
| @@ -1,11 +1,15 @@ | |||
| package nextstep.courses.domain; | |||
|
|
|||
| public interface EnrollmentRule { | |||
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.
인터페이스에 너무 많은 메서드를 제공하고 있음.
이름 또한 EnrollmentRule이라 수강 신청 규칙을 판단하는 메서드만 가질 것으로 판단되는데 규칙 외의 메서드도 가지고 있는 것으로 판단됨
인터페이스의 메서드를 최소화하기 위한 설계 개선을 시도해 본다.
| return jdbcTemplate.queryForObject(sql, (rs, rowNum) -> { | ||
|
|
||
| // 1️⃣ ImageFile (지금은 ID만 복원) | ||
| ImageFile imageFile = new ImageFile(rs.getLong("image_id")); |
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.
질문관련 피드백:
Session을 생성할 때 항상 ImageFile 상태 값을 함께 전달해야 한다면 image_file 테이블과 join한 후 ImageFile 값을 생성하는 것은 어떨까?
| status, | ||
| enrollmentRule, | ||
| enrollments | ||
| ); |
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.
이곳에서 모든 객체를 생성하기 보다 원시값, 문자열을 받을 수 있는 부 생성자를 추가한 후 생성자에서 객체를 생성하도록 추가하면 어떨까?
앞 단계의 미션에서 계속해서 연습한 내용임
| import java.time.LocalDateTime; | ||
| import java.util.Objects; | ||
|
|
||
| public class Session { |
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.
Service Layer를 담당하는 Service 클래스도 추가해 보는 것은 어떨까?
도메인 객체로 로직을 구현한 만큼 Service Layer가 로직을 담당하지 않는지?
Service 클래스가 Repository와 도메인 객체를 어떻게 연결하는지 확인해 보면 어떨까?
이번 기회에 Service Layer의 역할은 무엇인지 고민해 보면 어떨까?
|
피드백 반영하면서 질문 사항이 있습니다. 질문1sessionRepository의 find를 테스트를 할 때 imageFile을 만들고 저장한 뒤 session 객체를 만들고 저장하여 테스트 하였습니다. 이 과정에서 그 전 step에서 만들어둔 sessionBuilder를 사용하고 싶었지만 imageFile을 DB상 저장을 하지않아 오류가 뜨는 일을 발생하여 결국 사용하지않고 일일히 다 객체를 만들어서 사용했습니다. 이런 경우에는 어떤 방식으로 접근해야할지 궁금합니다. 질문2getPrice, getCapaciry 인터페이스에 있는거를 제거하고, session안에서 enrollmentRule.getType을 통해서 어떤 타입인지에 따라 반환값을 정의했는데 freeㄷEnrollmentRule에서는 null을 반환하게 했습니다. 이런 방법이 괜찮은 방법인지.. 혹은 다른 방법이 있는지 궁금합니다. 느낀점service layer를 만들면서 느낀점에 service에서는 로직이 이루어지는게 아니라 검증되고, 로직들을 조합하고 연결하는 역할을 하는것을 느꼈습니다. 이 느낀점이 맞는지 잘 모르겠습니다만..ㅎ 리뷰 잘 부탁드립니다. |
피드백 반영 및 step3 리뷰 요청드립니다.
질문 1
JdbcSessionRepository를 만들때 findById 메소드에서 고민에 빠졌는데요.
(rs, rowNum) -> new Session(
rs.getLong("id"),
rs.getLong("image_id"),
...
)
했을 때, session의 생성자에는 image_id가 아니라 imageFile 을 받고 있고, image_id를 받는 생성자를 만든다해도 결국에 imageFile을 생성하거나 불러와야하는데 이 부분을 어떻게 하는지 감이 안잡혀서 일단 각각의 인스턴스를 만들어서 new Session을 했는데요.
ImageFile imageFile = new ImageFile(rs.getLong("image_id"));
위와 같이 했는데 session에는 image_id만 가지고 있어서 어떻게 생성해야하는지.. 잘 모르겠어서 질문 남깁니다.