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

책을 등록하는 로직을 수정한다. #75

Merged
merged 2 commits into from
Feb 6, 2025
Merged

Conversation

f1v3-dev
Copy link
Member

@f1v3-dev f1v3-dev commented Feb 6, 2025

연관된 이슈

resolves #62

작업 내용

책 등록 (RegisterBookUseCase) 부분에서 이미 등록된 책은 새로 생성하지 않는 방향으로 수정하였습니다.

등록하려는 책이 존재하는 경우

Hibernate: 
    select
        me1_0.id,
        me1_0.created_at,
        me1_0.member_introduction,
        me1_0.member_book_public,
        me1_0.member_kakao_id,
        me1_0.member_nickname,
        me1_0.member_registration_id,
        me1_0.member_role,
        me1_0.updated_at 
    from
        member me1_0 
    where
        me1_0.id=?
Hibernate: 
    select
        be1_0.book_id,
        be1_0.book_author,
        be1_0.created_at,
        be1_0.book_published_at,
        be1_0.book_thumbnail_url,
        be1_0.book_title,
        be1_0.updated_at 
    from
        book be1_0 
    where
        be1_0.book_author=? 
        and be1_0.book_title=?
Hibernate: 
    insert 
    into
        member_book
        (book_id, member_book_completed_at, created_at, member_id, member_book_read_status, updated_at) 
    values
        (?, ?, ?, ?, ?, ?)

등록하려는 책이 없는 경우

Hibernate: 
    select
        me1_0.id,
        me1_0.created_at,
        me1_0.member_introduction,
        me1_0.member_book_public,
        me1_0.member_kakao_id,
        me1_0.member_nickname,
        me1_0.member_registration_id,
        me1_0.member_role,
        me1_0.updated_at 
    from
        member me1_0 
    where
        me1_0.id=?
Hibernate: 
    select
        be1_0.book_id,
        be1_0.book_author,
        be1_0.created_at,
        be1_0.book_published_at,
        be1_0.book_thumbnail_url,
        be1_0.book_title,
        be1_0.updated_at 
    from
        book be1_0 
    where
        be1_0.book_author=? 
        and be1_0.book_title=?
Hibernate:   # 책을 새로 등록한다. 
    insert 
    into
        book
        (book_author, created_at, book_published_at, book_thumbnail_url, book_title, updated_at) 
    values
        (?, ?, ?, ?, ?, ?)
Hibernate: 
    insert 
    into
        member_book
        (book_id, member_book_completed_at, created_at, member_id, member_book_read_status, updated_at) 
    values
        (?, ?, ?, ?, ?, ?)

리뷰 요구사항

Entity를 새로 생성하려고 . 을 눌렀을 때 Builder가 호출되는것을 보았는데 private 키워드로 막아둬도 생성이 되는 상황인데 이 부분에 대해서는 같이 한 번 찾아보면 좋을 것 같아요 !

@f1v3-dev f1v3-dev added the fix This issue or pull request already exists label Feb 6, 2025
@f1v3-dev f1v3-dev requested a review from hwangdaesun February 6, 2025 06:01
@f1v3-dev f1v3-dev self-assigned this Feb 6, 2025
@hwangdaesun
Copy link
Collaborator

생성자에 @builder를 사용하면, 해당 생성자에서 정의된 필드를 기반으로 하는 정적 내부 클래스를 생성하는 것으로 알고 있습니다.

해당 문서 참고해봐도 좋을 듯 합니당
https://projectlombok.org/api/lombok/Builder

Copy link
Collaborator

@hwangdaesun hwangdaesun left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!!

@f1v3-dev
Copy link
Member Author

f1v3-dev commented Feb 6, 2025

생성자에 @builder를 사용하면, 해당 생성자에서 정의된 필드를 기반으로 하는 정적 내부 클래스를 생성하는 것으로 알고 있습니다.

해당 문서 참고해봐도 좋을 듯 합니당 https://projectlombok.org/api/lombok/Builder

그렇다면, Entity를 외부에서 생성할 때 newInstance() 라는 정적 메서드로만 생성하고 싶게 하셨던 의도와는 조금 달라지지 않나요?

외부에서도 builder를 사용할 수 있게되는 것 같아서 이것까지 막아버리는게 맞나라고 생각이 들어요

@hwangdaesun
Copy link
Collaborator

hwangdaesun commented Feb 6, 2025

생성자에 @builder를 사용하면, 해당 생성자에서 정의된 필드를 기반으로 하는 정적 내부 클래스를 생성하는 것으로 알고 있습니다.
해당 문서 참고해봐도 좋을 듯 합니당 https://projectlombok.org/api/lombok/Builder

그렇다면, Entity를 외부에서 생성할 때 newInstance() 라는 정적 메서드로만 생성하고 싶게 하셨던 의도와는 조금 달라지지 않나요?

외부에서도 builder를 사용할 수 있게되는 것 같아서 이것까지 막아버리는게 맞나라고 생각이 들어요

이것까지 막아버리는 게 맞나 라는 말에서 "이것"이 무엇인 지 모르겠습니다. "이것"이 private 생성자인가요?
그리고, newInstance() 즉, 정적 팩토리 메서드를 사용하려는 이유는 해당 메서드가 엔티티를 생성한다 라는 이름을 가질 수 있게 하려는 의도였습니다.

아래 포스팅을 참고해도 좋을 거 같습니다.
https://tecoble.techcourse.co.kr/post/2020-05-26-static-factory-method/

@f1v3-dev
Copy link
Member Author

f1v3-dev commented Feb 6, 2025

현재 PR에 올라온 코드를 예시로 들자면

BookEntity를 생성하는 메서드

private BookEntity createBook(RegisterBookRequest request) {
    return BookEntity.newInstance(request.author(), request.title(), request.publishedAt());
}

여기에서 아래와 같이 호출을 해도 정상적으로 동작을 해요

private BookEntity createBook(RegisterBookRequest request) {
        
        return BookEntity.builder()
                .author(request.author())
                .title(request.title())
                .publishedAt(request.publishedAt())
                .build();
}

image

이런 방법처럼 @builder 어노테이션을 통해서 외부에서도 builder를 열어두게 되는건데 이렇게 되면 newInstance() 가 아닌 방법으로도 엔티티를 생성할 수 있는데 괜찮은가? 에 대해서 의견을 듣고싶었어요.

@hwangdaesun
Copy link
Collaborator

hwangdaesun commented Feb 6, 2025

현재 PR에 올라온 코드를 예시로 들자면

BookEntity를 생성하는 메서드

private BookEntity createBook(RegisterBookRequest request) {
    return BookEntity.newInstance(request.author(), request.title(), request.publishedAt());
}

여기에서 아래와 같이 호출을 해도 정상적으로 동작을 해요

private BookEntity createBook(RegisterBookRequest request) {
        
        return BookEntity.builder()
                .author(request.author())
                .title(request.title())
                .publishedAt(request.publishedAt())
                .build();
}

image

이런 방법처럼 @builder 어노테이션을 통해서 외부에서도 builder를 열어두게 되는건데 이렇게 되면 newInstance() 가 아닌 방법으로도 엔티티를 생성할 수 있는데 괜찮은가? 에 대해서 의견을 듣고싶었어요.

아 이해해습니다. 이건 약간 규칙에 관한 이야기인 거 같은데 저희가 커스텀하게 builder를 외부에 노출 시키지 않게 만들 지 않는 이상 다른 방법이 없지 않을까요? 함 찾아봐야겠네요.

아니면, @builder를 아예 사용하지 않는 방법 정도가 생각이 나네요. (다만,, @builder를 사용하면 원하는 필드만 사용해서 객체를 만들 수 있는 장점을 포기한다는 게 쉽지 않긴 하네요.. 필드 수가 적으면 생성자를 사용해도 괜찮을 수도 있겠네요)

@f1v3-dev
Copy link
Member Author

f1v3-dev commented Feb 6, 2025

image

방금 @Builder 옵션을 보니까 접근을 PRIVATE으로 막을 수 있네요? 이 방법을 사용하면 좋을 것 같아요 !

적용했을 때의 모습

image

@f1v3-dev f1v3-dev merged commit f7210ee into develop Feb 6, 2025
1 check passed
@f1v3-dev f1v3-dev deleted the fix/#62-book branch February 6, 2025 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix This issue or pull request already exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

책을 등록하는 로직을 수정한다.
2 participants