Skip to content

Conversation

dPwls0125
Copy link

안녕하세요 오찌:) JPA 미션 제출합니다.

미션을 진행하면서 궁금했던 부분은, "service 계층에서 다른 도메인의 repository를 모두 주입해 사용하는 것이 책임 분리의 관점에서 괜찮은 것인가?"입니다. 예를 들면 아래 reservation service에서 여러 도메인의 repository를 주입받아 사용했습니다.

public class ReservationService {

    private ReservationRepository reservationRepository;
    private ParticipationTimeRepository participationTimeRepository;
    private ThemeRepository themeRepository;
    private WaitingRepository waitingRepository;

Reservation 엔티티를 저장하는 과정에서 각 pk 값에 대한 객체를 찾아 저장하기 위함이었는데, 이 방식이 옳은 방식이었는지 의견을 구하고 싶습니다.

Copy link

@Ohzzi Ohzzi left a comment

Choose a reason for hiding this comment

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

수고하셨습니다 예진님! 잘 구현해주셨네요. 상세 사항에 대한 리뷰는 본문에 남겼고, 이 코멘트에서는 질문에 답변해드리도록 할게요!

미션을 진행하면서 궁금했던 부분은, "service 계층에서 다른 도메인의 repository를 모두 주입해 사용하는 것이 책임 분리의 관점에서 괜찮은 것인가?"입니다. 예를 들면 아래 reservation service에서 여러 도메인의 repository를 주입받아 사용했습니다.

사실 정답은 없습니다! 이 문제에 대해서도 각기 사람들마다 의견이 다르기도 하구요. 본인이 서비스가 다른 도메인의 서비스를 의존하든, 레포지토리를 의존하든 명확한 원칙과 이유가 있는 것이 중요하다고 생각해요.

제 개인적인 취향은 상하위 도메인 관계로 묶여있다면 레포지토리를 직접 써도 되지만, 아예 별개의 도메인이라면 레포지토리를 직접 쓰는 것을 선호하지는 않아요. 아무래도 각각의 도메인이 지켜야 할 비즈니스 로직은 엔티티 그 자체에도 있을 수 있지만 서비스의 비즈니스 로직으로도 갖춰져 있을 수 있거든요. 그런데 만약 레포지토리를 바로 쓰게 되면 그런 비즈니스 로직을 보장해줄 수 없기 때문이에요.


import java.util.Optional;

public interface MemberRepository extends CrudRepository<Member, Long> {
Copy link

Choose a reason for hiding this comment

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

오호, 많은 Repository 아류 인터페이스들이 있는데 Crud를 선택한 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

현재 상황에서는 페이징이나 정렬과 같은 기능이 필요하다 생각하지 않아, 기본적인 crud만 제공하는 인터페이스를 사용했습니다.

private String date;
private Time time;

@ManyToOne
Copy link

Choose a reason for hiding this comment

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

ManyToOne 안의 옵션을 보면 fetch 라는 옵션이 있어요. 기본이 EAGER로 되어있는데요, EAGER와 LAZY의 차이가 뭘까요? 어떤 옵션을 선택하는게 좋을까요?

Copy link
Author

Choose a reason for hiding this comment

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

EAGER와 LAZY 의 차이는 언제 데이터를 언제 불러올 것인가의 시점 차이라고 생각합니다.

EAGER의 경우, JPA가 해당 엔티티를 조회할 때 연관 데이터를 즉시 로드하고, LAZY의 경우, 불필요한 데이터를 로딩하지 않고 실제 사용될 경우에 로딩합니다.

따라서 연관 데이터 전체가 반드시 사용되는 경우라면, 관련 데이터를 N번 쿼리하게 되는 N+1 문제를 방지하기 위해 즉시 로딩 전략을 사용하는게 좋을 것 같지만, 연관 데이터가 항상 사용되는 것이 아니라면 과도한 메모리 사용을 방지하기 위해 지연 로딩 전략을 사용하는게 좋을 것 같습니다.

return reservationService.findAll();
}

@GetMapping("/reservations-mine")
Copy link

Choose a reason for hiding this comment

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

optional) reservations 자체가 하나의 리소스로서의 역할을 가지고, mine은 그 하위 느낌이잖아요? 이런 경우 보통 하이픈 대신 슬래시로 계층을 더해서 URL을 구성하는 편이에요!

)));
}
}
//package roomescape.reservation;
Copy link

Choose a reason for hiding this comment

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

안쓰이는 클래스는 주석처리가 아닌 제거해주세요!


List<Reservation> findByMemberId(Long memberId);

List<Reservation> findAllByDateAndTheme_Id(String date, long themeId);
Copy link

Choose a reason for hiding this comment

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

오잉? Theme_Id 라는 명명법은 자바에는 없는 규칙입니다!

Comment on lines 45 to 55
List<WaitingWithRank> waitingsWithRank = waitingRepository.findWaitingsWithRankByMemberId(member.getId());

Long rank = -1L;
for (WaitingWithRank w : waitingsWithRank) {
if (w.getWaiting().getId().equals(waiting.getId())) {
rank = w.getRank();
break;
}
}
return new WaitingResponse(waiting.getId(), rank);
}
Copy link

Choose a reason for hiding this comment

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

이 부분에 쿼리도 그렇고 반복문 순회도 그렇고 비효율적인 부분이 존재하는 것 같아요! 개선할 수 있는 아이디어를 생각해볼까요??

Copy link
Author

Choose a reason for hiding this comment

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

멤버의 모든 대기에 대한 쿼리를 하고 또다시 반복문으로 필터링 할 것이 아니라, 현재 대기(멤버, 대기 신청한 날짜, 시간, 테마)에 대한 대기의 순위만 쿼리하면 되었던 간단한 문제였습니다.

아래와 같이 쿼리를 수정하고 반복문을 제거하였습니다.

    @Query("SELECT (SELECT COUNT(w2) " +
            "     FROM Waiting w2 " +
            "     WHERE w2.theme = w.theme " +
            "       AND w2.date = w.date " +
            "       AND w2.time = w.time " +
            "       AND w2.id <= w.id) " +
            "FROM Waiting w " +
            "WHERE w.id = :waitingId")
    Long findRankOfWaiting(Long waitingId);
}

Comment on lines +33 to +44
Member member = memberRepository.findById(loginMember.getId()).orElseThrow(() -> new IllegalArgumentException("존재하지 않는 회원입니다."));
ParticipationTime participationTime = participationTimeRepository.findById(waitingRequest.time()).orElseThrow(() -> new IllegalArgumentException("존재하지 않는 시간입니다."));
Theme theme = themeRepository.findById(waitingRequest.theme()).orElseThrow(() -> new IllegalArgumentException("존재하지 않는 테마입니다."));

Waiting waiting = new Waiting(
member,
waitingRequest.date(),
participationTime,
theme
);

waiting = waitingRepository.save(waiting);
Copy link

Choose a reason for hiding this comment

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

여기서 member, participationTime, theme을 조회할 때 각각 select가 한 번씩 나갔을텐데요, 혹시 waiting 저장할 때도 select가 또 나가지 않을까요?

Copy link
Author

Choose a reason for hiding this comment

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

waiting 을 저장할때 insert 쿼리만 실행할것이라고 생각했는데, select가 또 나가게 되는건가요? 잘 이해되지 않습니다.

@ManyToOne
@JoinColumn(name = "member_id")
private Member member;
private String date;
Copy link

Choose a reason for hiding this comment

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

요기는 컬럼 어노테이션이 빠졌네요?

Copy link
Author

Choose a reason for hiding this comment

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

필드명이랑 동일하게 사용하면 될것이라고 생각해서 컬럼어노테이션을 붙이지 않았는데,
전체 필드에 컬럼 어노테이션을 붙여주어야할 이유가 무엇일까요?

Comment on lines +10 to +12

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
Copy link

Choose a reason for hiding this comment

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

다른 필드들에도 컬럼 어노테이션이 붙는 것이 좋지 않을까요?

@RestController
public class ThemeController {
private ThemeDao themeDao;
private final ThemeRepository themeRepository;
Copy link

Choose a reason for hiding this comment

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

컨트롤러가 서비스를 건너뛰고 레포지토리를 바로 사용한다면 어떤 장단점이 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

사실 Service 계층을 사용했을때의 장점은 비즈니스로직 분리, 재사용성 높아짐 말고는 아직 잘 모르곘습니다.
서비스 계층을 이용하지 않으면 트랜잭션 관리 문제도 있을까요?

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