-
Notifications
You must be signed in to change notification settings - Fork 134
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
[Spring MVC + JDBC + CORE] 이해찬 미션 제출합니다. #369
base: main
Are you sure you want to change the base?
Conversation
# Conflicts: # build.gradle # src/test/java/roomescape/MissionStepTest.java
refactor: Reservation 필드 (date, time) 분리
# Conflicts: # src/main/java/roomescape/dao/ReservationDao.java # src/main/java/roomescape/service/ReservationServiceImpl.java # src/main/resources/application.yml # src/test/java/roomescape/MissionStepTest.java
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.
- 처음에는 LocalDateTime으로 내부적으로 저장하였는데, 추후 String으로 저장하여라고 해서 리팩토링을 하였습니다. 보통 DB에 날짜와 시간 정보를 합쳐서 스트링이 아닌 DATETIME형식으로 작성하지 않나요..?
음ㅁ 사실 DATETIME을 더 많이 쓰긴합니다. 미션에서는 복잡도를 낮추기 위해서 단순하게 String으로 구성한 것 같아요.
- Controller를 보통 엔티티 별로 만들었는데, HomeController가 걸렸습니다. Controller는 화면이나 주소 단위로 작성하는지 아님 엔티티 단위로 작성하는지 궁금합니다.
요건 아래에서도 이야기를 하겠지만, 해찬님이 생각하는 Entity란 무엇인가가 궁금합니다.
그거랑 별개로 Controller가 어떤걸 나타내기 위한 클래스인지에 대해 고민해보면 엔티티단위로 나오는지 화면 단위로 나오는지 아니면 다른 기준이 있는지 알 수 있으실 것 같습니다.
RestFul에 대해서 알아보시면 도움이 될거다.
3.Exception을 자세하게 적어본 적이 없는데, Exception을 기능단위로 만드는지, 엔티티(도메인) 단위로 만드는지 궁금합니다.
Exception을 이번에 도메인 단위로 잘 만들어주셨더라고요. 그거는 사람마다 다릅니다.
저는 도메인 단위로 만드는 걸 선호하는데 그렇지 않은신 분들도 많더라고요.
이 기준은 정하기 나름입니다. 대신 응집성 있게 잘 모아두긴 해야겠죠?
- 도메인과 엔티티는 개념상 어떤 차이가 있는지 궁금합니다. 제가 도메인이라고 작성한 각 클래스들을 엔티티라고 해야하는지, 도메인이라고 해야하는지 헷갈립니다..
요거는 간단하게 말하기 어려운 주제이고, 스스로 찾아보면 좋은 부분인 것 같습니다. 이거에 대해서는 일요일 스터디 때 한번 이야기를 나눠보죠.
- 이전에 spring을 공부했을 땐 converter를 만들었었는데, 사용되는 코드가 한정적이고 특별한 메소드가 포함안되는 것 같은데 꼭 만들어야 하는건지 궁금합니다.
converter라는 클래스는 저는 이번에 처음봤는데요. 어떤 누군가의 코드 스타일 중 하나겠죠?
해찬님이 필요없다고 생각하신 다면 없애셔도 됩니다. 단 그러면 기존 converter가 갖고 있었던 책임은 어디로 가야하는지? 그 책임을 지는 클래스는 어떤 식으로 동작해야할까요?
질문하신 부분들에 대한 몇몇 코멘트 달았습니다.
고생하셨습니다. 내일 스터디 때 같이 이야기해보면 좋을 것 같아요.
import roomescape.domain.Reservation; | ||
import roomescape.dto.ReservationReqeustDto; | ||
|
||
public interface ReservationService { |
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 Interface를 만드신 이유는 무엇인가요??
interface는 어떨 때 만들어야 하나요?
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.
늘 의문이 드는 부분이긴 하나, 사람들이 interface를 만드는덴 이유가 있다고 믿었습니다.
사실상 코드반복이 없고 추상화를 해야하나 싶기도 하지만 찾아보면 interface를 쓰는덴 나름의 이유가 있긴 한 것 같더라고요..
아직 의존성이나 클래스에 대한 개념이 부족하여 관련글을 제대로 이해 못 했는데, 일단 실무에서 작성하는 방식에 익숙해지자는 마인드로 interface를 넣었습니다.
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.
이 Interface를 만드는 이유는 이전에 Spring가 쓰이기 전엔 interface로만 의존성 주입이 가능해서 만든 걸로 알고 있습니다.
지금은 interface가 없어도 의존성 주입이 되죠? 이 service interface를 만들 필요는 없습니다.
저는 interface라는 자원을 좀 귀중히 여기는데요. 추상화 레벨이 올라가면서 디버깅도 조금 어려워지는 역할도 해요.
어차피 구현체가 하나밖에 존재 안할거라면 빼는 걸 추천드립니다.
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.
의문을 이전부터 가지고 계셨다니 좋네요. 하지만 거기서 더 나아가서 의심해보면 더 좋을 것 같아요.
@Override | ||
public Time delete(Long id) { | ||
|
||
Time time = this.findById(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.
조회하고 삭제하는 식으로 코드를 구성하셨군요.
이 경우 쿼리가 2번 나갈 것 같아요. 다른 방법은 없을까요?
있다면, 이 방법을 선택하신 이유가 있을까요?
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.
뒤에 TimeDao.delete를 호출하는데 이 메소드는 말 그대로 삭제만을 담당하게 하고 싶었습니다. 실제로 저 id의 Time 값이 있는지는 findById()에서 검증하며 예외처리 하고 싶어서 저렇게 짰습니다. 쿼리조회는 O(1)이라서 2번 조회하지 했었습니다..
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.
사실 말씀하신 것처럼 쿼리가 그렇게 많이 나가는 편은 아니에요. 요정도면 그냥 사용하셔도 됩니다.
이 질문을 남겨드린 의도는 jdbc의 update 메서드에 대해서 알려드리고 싶은 거였습니다.
jdbc에서는 update 메서드를 사용했을 때 영향받은 row의 수를 알려줘요.
그를 이용한다면 쿼리 한번에 삭제된 것이 몇개인지도 알수 있겠죠?
public class ReservationServiceImpl implements ReservationService { | ||
|
||
private final ReservationDao reservationDao; | ||
private final TimeService timeService; |
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가 service를 참조하는 구조는 괜찮을까요?
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.
계층구조에서는 같은 계층인 다른 요소에 접근 불가능한 것이 맞는 건가요?
저기선 Time의 정보를 가져와야 하고, 그럼 하위 계층인 TimeDao를 참조해야 하는 것일까요?
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가 Service를 참조하는 것은 레이어드 아키텍쳐에 따르면 가능하다곤 하나, 전체 클래스 다이어그램을 보았을 때 이쁘지가 않아서 그냥 controller에서 Service를 참조하도록 고쳤습니다
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.
계층 구조에서 같은 계층 구조를 참조 하는 것은 원칙적으로는? 불가능 한게 맞다고 생각합니다. 그런게 잦다보면 순환 참조가 일어나기 쉽거든요.
그치만 본인만의 기준이 확실하다면 참조하는 것도 괜찮다고 생각해요
|
||
@ControllerAdvice | ||
public class | ||
ErrorHandler { |
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.
예외처리를 위한 ControllerAdvice 좋습니다
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.
쓰는 법을 잘 몰라 반쪽짜리 Advice가 된 것 같습니다. 예외를 던질때 Enum으로 (상태코드, 문장)을 던지고 그에 맞춰 응답을 보내고 싶은데.. 구조가 잘 이해되지 않습니다.
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.
해당 구조는 제가 나중에 설명드리도록 하겠습니당~
FEAT: ErrorHandler 보완 (작동 안됨) 실행에는 문제 없으나 ErrorHandler에서 원하는 문구를 출력해주지 않음
@@ -0,0 +1,31 @@ | |||
#-------------------------------------------------------------------------------# |
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.
오 이건 어떤 역할을 하는 yaml인가요??
} | ||
|
||
@GetMapping("/reservations") | ||
public ResponseEntity<?> reservations() { |
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.
와일드카드를 사용하신 이유가 있으신가요???
|
||
) { | ||
|
||
public Reservation toReservation(Time time) { |
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.
오 요런식으로 dto에서 도메인 객체 만드는 거 좋은 것 같습니다.
public ResponseEntity<?> createReservation (@RequestBody @Valid ReservationRequestDto request) { | ||
|
||
Time time = timeService.findById(request.time()); | ||
Reservation reservation = reservationService.create(request.toReservation(time)); |
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.
요런 방식도 괜찮은 것 같네요
}; | ||
|
||
|
||
public Optional<Reservation> findById(Long 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.
Optional도 잘 써주셨군요!! 좋습니다.
@Override | ||
public Reservation findById(Long id) { | ||
|
||
return reservationDao.findById(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.
되게 잘 짜주신것 같습니다.
10단계에 삭제가 제대로 작동하지 않는 것을 제외한 나머지 부분을 완성하여 일단 제출해봅니다..똑같은 쿼리문을 h2에서 작성했을 땐 작동되었는데, 코드상으로 나타내니 에러가 계속 떴습니다.아래는 10단계까지 구현해보며 의문이 든 부분이나 부족한 부분 질문으로 남겨보았습니다.
[추가] (11/15)
<전체 구조도>
이상 있던 부분도 해결했고, ErrorHandler는 거의 작동은 안되지만 구현은 해보았습니다..
(이전엔 쿼리로 JOIN 연산 후 컬럼명이 일치하지 않아 데이터 조회를 못하는 에러가 있었네요.)
질문은 대부분 해소 되었습니다. 이미 구현한 만큰 다른 걸 추가로 해보자 하다가 블로그를 쓰기 시작했습니다. 열심히 써보고 있는데 싶지 않네요.. 코드 위주로 쓰셨나요? 아님 잡다한 고민들 위주로 쓰셨나요? 블로그에 전체 코드를 붙여놓지 말아야 할지 고민입니다.
https://velog.io/@lhc0312/Spring-%EB%B0%A9%ED%83%88%EC%B6%9C-%EC%96%B4%EB%93%9C%EB%AF%BC-%EC%95%A0%ED%94%8C%EB%A6%AC%EC%BC%80%EC%9D%B4%EC%85%98
다 못 적어서 비공개지만 언젠간 공개 되겠죠.??
-- 질문 --
처음에는 LocalDateTime으로 내부적으로 저장하였는데, 추후 String으로 저장하여라고 해서 리팩토링을 하였습니다. 보통 DB에 날짜와 시간 정보를 합쳐서 스트링이 아닌 DATETIME형식으로 작성하지 않나요..?
Controller를 보통 엔티티 별로 만들었는데, HomeController가 걸렸습니다. Controller는 화면이나 주소 단위로 작성하는지 아님 엔티티 단위로 작성하는지 궁금합니다.
3.Exception을 자세하게 적어본 적이 없는데, Exception을 기능단위로 만드는지, 엔티티(도메인) 단위로 만드는지 궁금합니다.
도메인과 엔티티는 개념상 어떤 차이가 있는지 궁금합니다. 제가 도메인이라고 작성한 각 클래스들을 엔티티라고 해야하는지, 도메인이라고 해야하는지 헷갈립니다..
이전에 spring을 공부했을 땐 converter를 만들었었는데, 사용되는 코드가 한정적이고 특별한 메소드가 포함안되는 것 같은데 꼭 만들어야 하는건지 궁금합니다.
-- 부족한 부분 --
ExceptionAdvice를 구현하는 것이 좋다고 생각했는데, 제대로 구현이 안된 것 같습니다.
일단계부터 십단계까지 단계적으로 구현하다보니 제 생각과 요구사항이 틀어질때가 있어 코드 스타일이 조금씩 바뀌었습니다. 빠르게 리팩토링을 거쳐 통일성있게 만들겠습니다..
SQL 문법에 익숙하지 않은데, 조금 찾아봐야할 것 같습니다..