Skip to content
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] 이동호 미션 제출합니다. #264

Open
wants to merge 21 commits into
base: plusultracode
Choose a base branch
from

Conversation

PlusUltraCode
Copy link

@PlusUltraCode PlusUltraCode commented May 27, 2024

코드 리뷰어님 한주가 고생 많으셨습니다 : )

코드 계층화에 신경써 가면서 요번 미션을 수행했습니다.

다만

코드가 계속 실행이 안됩니다...

오류 이유
Validation failed for argument [0] in public org.springframework.http.ResponseEntity<roomescape.reservation.dto.ReservationResponse> roomescape.reservation.controller.ReservationApiController.addReservation(roomescape.reservation.dto.ReservationRequest): [Field error in object 'reservationRequest' on field 'timeId': rejected value [null]; codes [NotNull.reservationRequest.timeId,NotNull.timeId,NotNull.java.lang.Long,NotNull]; arguments [org.springframework.context.support.DefaultMessageSourceResolvable: codes [reservationRequest.timeId,timeId]; arguments []; default message [timeId]]; default message [널이어서는 안됩니다]]

해석해보니 reservation 에서 timeId가 들어오지 않았다고 합니다.

9단계 10단계 테스트를 진행해 봤는데 테스트는 정상 적으로 진행 되었습니다.

혹시 아시는 부분이 있을까요?? 다른 분들의 코드를 참고 및 수정도 해봤지만
계속 위와 같은 오류가 뜨네요

오류를 발견하시는것도 좋지만

코드의 계층화 역할 분담 등 요번 미션 수행을 잘 지켰는지 코드리뷰 부탁드려요 : )

Core commit입니다!!

@mangsuyo
Copy link

안녕하세요 동호님.
저도 미션을 수행하다가 이 문제를 겪어서 혹시 도움이 될까 적어봅니다!
아마도 클라이언트에서 time : Long의 형식으로 Request를 보내는 것 같아요. reservation RequestDTOtimeId -> time으로 필드 명 바꾸시면 해결될 것 같습니다!

@sangu1026
Copy link

안녕하세요 동호님 우선 미션하시느라 고생 많으셨습니다 👏
저도 테스트가 모두 통과하여서 잘 몰랐었는데 실제로 reservation을 등록하는 페이지에서
등록이 잘 안되는 문제가 발생하더라구요...

저도 ReservationRequestDto에서 timeId로 time의 id를 받았었는데 알고보니
time으로 받아야지 컨트롤러에서 @RequestBody로 ReservationRequestDto에 매핑이 가능하더라구요.
덕분에 저도 알지 못하는 오류를 고칠 수 있었습니다. 감사합니다!

@NotBlank
private String date;

@NotBlank

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 timeId가 아닌 time으로 수정해야 될 것 같습니다

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

감사합니다 상우님!!
TimeEntity클래스 명을 Time 으로 바꾸니 다른 클래스에서 TimeEntity time~~~ 라고 선언한 변수면들이 다 바뀌어
프로그램이 정상 작동하게 되었습니다.
오류를 찾기위해 많은 시간을 써서 그런지 매핑의 중요성으 뼈저리게 느꼈습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맞습니다. 저도 스프링 미션을 하면서 여러가지 오류들 때문에 꽤 고생을 했었습니다.
오류들을 해결하는 과정에서 오류를 아예 안내는 것도 중요하지만 오류가 났을때
그 오류의 원인을 빠르게 찾고 수정하는 것도 정말 중요한 것 같습니다. 수고하셨습니다!👍

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 Service가 Repository를 호출하는 해주는 역할밖에 할 수 없어서
Service를 도입하는게 역할 분리에 큰 의미가 없다고 생각했었습니다.
그런데 다시 생각해보니 컨트롤러가 직접적으로 Repository를 호출하고 의존하는 것은 컨트롤러에 맞는
역할이 아닌거 같아 동호님처럼 Service를 도입하는 것이 맞는 것 같습니다.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 서비스의 존재 이유를 몰랐지만 직접 사용해보니/
서비스는 컨트롤러와 Repository 사이에 있는 다리와 같다는 느낌을 많이 받았습니다.

컨트롤러 입장에서 서비스에게 명령만 해주고
나머지는 서비스와 Repository 두 객체 들끼리 해결한 뒤
정보만 컨트롤러에게 전달하는 느낌이라 생각합니다.

Comment on lines 8 to 11
@Data
@NoArgsConstructor
@AllArgsConstructor
@NoArgsConstructor
@Builder

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

생성자를 사용하지 않고 @builder와 같은 어노테이션으로 객체를 생성할 수 있군요.
혹시 @builder를 사용하신 이유가 단순히 생성자 코드를 생략하기 위해서 이신가요?
아니면 혹시 다른 이점이 있나요??

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReservationEntity.builder()
.id(id2)
.name(name2)
.date(date2)
.time(time2) .build();
와 같은 형식으로 빌더를 작성합니다.

빌더를 사용하지 않고 해당 ReservationEntity 클래스에
따로 생성자를 선언해주어도 가능합니다.
그렇게 되면 new ReservationEntity(id2,name2,date2,time2); 와 같은 형식으로 만들어야 됩니다.

이런 상황이면 문제가 되지 않지만
만약에 date2 나 time2 에 값을 NULL값으로 하고 싶을 경우가 생겼다고 해봅시다.

클래스를 수정하지 않았다고 가정하고
new ReservationEntity(id2.name2) 와 같이 선언을 하게 되면 오류가 생깁니다.
즉 어떤 경우가 생길때마다 필요에 맞는 생성자를 추가하거나
아니면 default생성자를 만든뒤
reservationEntity.setId(id2);
reservationEntity.name(name2);와 같이 초기화 해야 됩니다.

다만 builder를 사용하게 되면
ReservationEntity.builder()
.id(id2)
.name(name2)
.build();

와 같이 선언만 하게 되면 자동으로 date와 time 은 null값으로 초기화 되는 간편함이 있습니다.

또한 builder를 사용하면 가독성이 높다고 생각합니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 저번 미션을 진행할 때 default 생성자를 만들지 않아 오류가 발생하기도 하고,
null값을 넣어줘야할때마다 해당 생성자를 직접 만들었는데,
builder를 사용하면 이러한 문제도 해결하고 가독성도 높여주는 효과가 있었군요!
다음번에 저도 builder를 적용시켜서 코드를 작성해보도록 하겠습니다. 감사합니다 👍

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 @GetMapping(value = "/reservations"), @PostMapping(value = "/reservations"), @DeleteMapping(value = "/reservations/{id}") 부분에서 "/reservations"가 중복되는걸 줄여주고 싶어서
@controller 어노테이션 위에 RequestMapping("/reservations")를 적어주었습니다.

이렇게 하면 중복되는 부분은 제거되어
@GetMapping(value = "/reservations") -> @GetMapping
@PostMapping(value = "/reservations") -> @PostMapping
@DeleteMapping(value = "/reservations/{id}") -> @DeleteMapping("/{id})
조금 더 간결하게 짤 수 있었던 거 같아요.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RequsetMapping("/reservaations")를 사용하니
해당 클래스 안에 있는 @GetMapping @PostMapping에
중복되는 "/reservations"을 없앨 수 있게 되었습니다.

감사합니다 ^^

@Builder
public class TimeRequest {

@NotBlank

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@notblank 어노테이션을 통해서 간단히 검증을 할 수 있다는 것을 알게되었습니다. 감사합니다!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@notblank null인지 검사 및 공백 검사 및 빈칸 검사
@NotNull null인지 검사
@notempty null인지 검사 및 빈칸 검사

등 어노테이션을 통해 간단하게 검증하도록 구현되어 있는게 있습니다.

@notblank 를 사용하면 자동으로 null, 공백, 빈칸 3가지를 검증해주기 때문에
@notblank 를 사용하는 편입니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NotNull, @notempty 라는 애노테이션도 있었네요. 추가적인 설명 감사합니다 👍

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

컨트롤러, 서비스, 레파지토리로 역할을 나누어 코드의 계층화 역할 분담을 확실히 잘 하신 것 같아요.
지난 미션이긴하지만 Entity와 Dto도 잘 분리되어 있어 조금 더 안정화된 객체 전달이 되는 것 같습니다.
미션하시느라 고생하셨습니다!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

상우님의 리뷰가 많은 도움이 되었습니다. 감사합니다 : )

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다! 남은 미션들도 화이팅입니다 👊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants