-
Notifications
You must be signed in to change notification settings - Fork 133
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 Core] 홍석우 미션 제출합니다. #381
base: stonecau
Are you sure you want to change the base?
Conversation
- controller, domain 폴더 생성
- JDBC 연동 - 데이터베이스명 검증 - 테이블명 검증
- ReservationController: - @RestController 어노테이션 변경 - JdbcTemplete 의존성 주입 - getReservations 메서드 변경
- addReservation: keyHolder를 통해 URI 생성 - deleteReservation: jdbcTemplete로 제거하도록 변경
- Time 객체 추가 - TimeController, TimeRequestDto 추가 - schema.sql 업데이트
- Controller 수정 - Reservation에 Time 객체 추가 - schema.sql 수정 - TimeTest 9단계 수행
- 두 객체를 처리하기때문에 변경
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.
몇몇 부분들에 대해서 코멘트 남겼어요.
확인 해주세요!!!
언제나 반박, 토론, 질문 환영입니다.
} | ||
|
||
@GetMapping("/reservations") | ||
public ResponseEntity<List<Reservation>> getReservations() { |
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.
도메인 객체를 직접적으로 반환하고 있군요.
이런 경우 도메인이 view에 대한 책임을 갖게되기도 합니다.
한번 dto를 도입해서 의존을 끊어보는 건 어떨까요?
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.
해당 결과에 대한 responseDto를 만들어서 반환하면 좋을 것 같네요! 감사합니다
|
||
private final TimeDao timeDao; | ||
|
||
@Transactional |
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.
이 Transactionoal 어노테이션은 붙이게되면 어떤 현상이 발생하나요?
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의 데이터 변동이 생겼을 때, commit과 rollback을 담당하는 기능 같네요!
이 메서드와 같이 단순히 데이터를 찾는 것이라면 딱히 필요 없는 것이 맞을까요?
|
||
return jdbcTemplate.query(sql, | ||
(rs,rowNum) -> new Time(rs.getLong("id"), rs.getString("time")), timeId) | ||
.stream().findFirst().orElse(null); |
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에서 orElse를 사용해주셔서 Time객체로 반환시키고 있군요.
Optional은 어떨 때 사용 되고, orElse를 쓰신 이유는 무엇인가요?
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은 해당 값이 null인지 아닌지 알 수 없을 때 사용되는 것으로 알고있습니다
리턴 타입을 Optional과 같이 지정해주었다면 stream이 orElse 없이 사용될 수 있겠군요
이런 방식이 더 효율적인 방법일까요? 아니면 가독성면에서 더 좋은 방법일까요?
|
||
@Getter | ||
@Builder | ||
@AllArgsConstructor |
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.
이 AllArgs와 NoArgs는 모두사용하는 생성자인가요?
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.
무지성으로 붙이는건 좋은 방법이 아니라고 수업시간에도 언급하셨던것 같은데
주의해서 쓰도록 해봐야겠네요 ㅠㅠ
@@ -0,0 +1,8 @@ | |||
package roomescape.dto; | |||
|
|||
public record ReservationRequestDto( |
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.
record를 써보셨군요. 써보시니 어떤가요? 괜찮은 것 같나요?
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 class GeneralExceptionHandler { | ||
|
||
@ExceptionHandler(IllegalArgumentException.class) | ||
public ResponseEntity<String> handleException(IllegalArgumentException e) { |
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.
이 ExceptionHandler는 정상적으로 동작하던가요?
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에서 나온 exception이라도 결국에 Controller에서 일어난 exception이니까
Handler에서 감지가 될 줄 알았는데 지금해보니까 안되네요..
음... 원인을 모르겠네요
} | ||
|
||
public List<Reservation> findAll() { | ||
String sql = "SELECT \n" |
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.
자바 17부터는 여러줄에 해당하는 문자열을
string sql = """
select *
from abc
where djklfkdjsafl
"""
이런식으로 묶어서 처리할 수 있습니다. 이를 활용해보면 좀 더 가독성 좋은 코드를 작성할 수 있을 것 같아요
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.
확실히 가독성이 좋아지는 것 같네요!
validateTime(request.time()); | ||
} | ||
|
||
private void validateName(String name) { |
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.
입력에 대한 validation과 service의 validation이 섞인 것 같아요.
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.
입력에 대한 검증은 requestDto에서 처리해볼 수 있을 것 같아요
public Reservation addReservation(ReservationRequestDto request) { | ||
validate(request); | ||
|
||
Time time = timeService.findById(request.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.
time이 null일 수도 있을 것 같아요. 이런 경우 어떤 일이 발생할까요?
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이 없는 reservation이 만들어 질 수 있을 것 같아요
예외처리를 추가하겠습니다!
Jdbc랑 Dao가 많이 생소해서 오래걸렸네요...ㅎ
그래도 오랜시간 투자해서 코드를 짜보니까 레이어드 아키텍처에 대한 이해가 깊어진 것 같습니다!
하면서 어려웠던 부분이나 생소했던 부분은 내일 정리해서 가져오겠습니다!!