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 JDBC] 신예린 미션 제출합니다. #255

Open
wants to merge 40 commits into
base: nyeroni
Choose a base branch
from

Conversation

nyeroni
Copy link

@nyeroni nyeroni commented May 20, 2024

JDBC 코드

  • 링크 안에 있는 커밋들만 코드 리뷰해주시면 될 거 같아요! :) 편하게 남겨주세용

Copy link

@mangsuyo mangsuyo left a comment

Choose a reason for hiding this comment

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

안녕하세요 예린님.
미션 수행하느라 고생 많으셨습니다 👍

서비스 로직의 변경을 최소화하기 위해 저장소를 추상화한 부분이 인상 깊습니다.
전체적으로 코드의 책임과 역할이 분리되어 있고 읽기 수월했습니다 :)

코드 리뷰는 상호간에 성장하고 다른 사람의 관점을 배울 수 있는 시간이라고 생각합니다.
리뷰에 답변 달아주시면 감사하겠습니다 :)

build.gradle Outdated
Comment on lines 22 to 24
compileOnly 'org.projectlombok:lombok'
annotationProcessor 'org.projectlombok:lombok'
testAnnotationProcessor 'org.projectlombok:lombok'

Choose a reason for hiding this comment

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

5주차 Spring MVC 미션의 공통 피드백에서 언급된 부분이라 여쭤봅니다.
예린님은 Lombok 의 어떤 기능 때문에 의존성을 추가하셨나요?
사용했을 때 장점과 단점은 뭐라고 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

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

getter, setter, toString, equals, hashCode 등의 메서드를 자동으로 생성할 수 있어 코드가 간결해지고 가독성도 좋아져서 자주 사용합니다. 단점은 딱히 없다고 생각하는데 있으시다면 알려주시면 감사하겠습니다!

Choose a reason for hiding this comment

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

예린님이 말씀하신 것처럼 편리하게 사용할 수 있고 일관되며 가독성을 높일 수 있다는 것이 Lombok의 장점이라고 생각합니다.

밑에 리뷰를 남긴 것처럼 Reservation@Data로 선언되어 외부에서 setter를 통해 값이 변경될 수 있다는 점을 단점이라 생각했어요!

Lombok을 사용할 때 주의점을 담은 좋은 글을 소개해드릴게요 :)
https://kwonnam.pe.kr/wiki/java/lombok/pitfall


import java.util.List;

public interface ReservationRepositoryImpl {
Copy link

@mangsuyo mangsuyo May 21, 2024

Choose a reason for hiding this comment

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

저장소가 변경되더라도 service의 코드는 변경되지 않도록 추상화 하신 것 같아요.
인터페이스 이름인 ReservationRepsoitoryImpl에서 Impl은 어떤 걸 의미하나요?

Copy link
Author

Choose a reason for hiding this comment

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

interface에 차이를 주기 위해 implement의 줄임표현인 Impl를 사용했습니다!

Copy link

@mangsuyo mangsuyo May 24, 2024

Choose a reason for hiding this comment

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

인터페이스를 구현하는 클래스에 Impl을 붙이는 일반적인 관행이 있어서 궁금해서 여쭤봤습니다!
InterfaceName + Impl 로 구현클래스 이름을 지으면 어떤 인터페이스를 구현한 클래스인지 알려주는데 더 유리하다고 생각해요 :)

}, keyHolder);

Long generatedId = keyHolder.getKey().longValue();
reservation.setId(generatedId);

Choose a reason for hiding this comment

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

도메인 reservation은 모든 필드에 setter가 있기 때문에 저장소 뿐만 아니라 어디에서도 필드에 접근하여 다른 값으로 변경 할 수 있네요.
예린님은 setter의 사용에 대해서 어떻게 생각하시는지 궁금합니다 :)

Copy link
Author

Choose a reason for hiding this comment

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

원랜 엔티티에서 setter를 사용하지 않는데, 이번엔 잘못 넣은 것 같네요! Reservationsetter를 두지 않고 필요할 때 추가하는 것이 맞다고 생각합니다!

Comment on lines 23 to 24
public List<ReservationResponse> findAllReservations() {
List<Reservation> reservations = reservationRepository.findAll();
List<Reservation> reservations = reservationJdbcRepository.findAll();
Copy link

@mangsuyo mangsuyo May 21, 2024

Choose a reason for hiding this comment

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

저장소 로직을 추상화하여 서비스 로직의 변경을 최소화 하신점이 인상 깊습니다. :)
그러나, 서비스 계층에서 어떤 저장소를 사용할지 선택하기 위해서 service의 코드가 변경되네요.

구현체를 주입받을 때 @Primary와 같은 우선순위를 지정하는 어노테이션을 사용하면 구현체를 지정할 때 코드 변경을 최소화 할 수 있습니다!

학습해보시면 좋을 것 같습니다 :)

Copy link
Author

Choose a reason for hiding this comment

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

@Primary 적용해서 바꿨습니다! 이 기능에 대해서는 더 공부해볼게요!

Comment on lines 39 to 40
reservation.setDate(date);
reservation.setTime(time);

Choose a reason for hiding this comment

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

객체를 생성자로 생성하지 않고 set 방식을 사용하신 이유가 궁금합니다 :)

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.

Reservation reservation = new Reservation();
reservation.setName(name);
reservation.setDate(date);
reservation.setTime(time);
혼동이 있었나봐요, 위의 코드에 대해 질문드렸습니다..!

Reservation reservation = new Reservation(name, date, time) 방식으로 생성하지 않으신 이유가 궁금합니다!

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.

2 participants