✨ [Code Review] 동동 code review

[dogsub]

No description provided.

[strongmhk]

고생하셨습니다👏

[strongmhk]

Q 파일은 시스템이 자동으로 생성해주는 파일들이라서, 라이브러리 버전이 올라감에 따라서 세부적인 사항들이 변경될 수 있습니다.
git에서 관리하면 버전 향상에 따른 변경 사항까지 commit 해야하기 때문에 ignore하시는게 좋아보입니다!

[strongmhk]

워크북에서는 클래스 단에 @AllArgsConstructor와 @builder를 같이 사용하였는데, 저는 엔티티 단에 @AllArgsConstructor 적용하는 것을 지양하는 편입니다
그 이유는 다음과 같습니다!

첫번째로, Agreement 클래스 단에 @builder와 @AllArgsConstructor를 붙이면 외부에서(Controller, Service 계층 등) PK인 id값까지 세팅할 수 있도록 노출됩니다!
PK값은 Auto Increment 전략을 사용한다면 기본적으로 JPA가 그 값을 관리해주는데, 모든 필드에 대한 생성자가 오픈된다면 불변성을 만족하지 못하게 되고, 제일 중요한 데이터 무결성이 깨져버릴 수 있습니다.

두번째로,@AllArgsConstructor를 사용하는 경우 해당 객체를 생성할 때 파라미터의 순서가 실수로 변경되어도 컴파일 에러가 발생하지 않고, 추후 인지하지 못한 치명적인 버그가 발생할 수 있는 여지가 됩니다.
(참조: https://www.inflearn.com/questions/1179578/%ED%98%B9%EC%8B%9C-allargsconstructor-%EB%A5%BC-%EC%A7%80%EC%96%91%ED%95%98%EC%8B%9C%EB%8A%94-%EC%9D%B4%EC%9C%A0%EA%B0%80-%EB%B9%8C%EB%8D%94-%ED%8C%A8%ED%84%B4%EC%9D%84-%EC%82%AC%EC%9A%A9%ED%95%98%EA%B8%B0-%EC%9C%84%ED%95%A8%EC%9D%B8%EA%B0%80%EC%9A%94,
https://obv-cloud.com/35)

그래서 결론적으로는, 클래스단에 붙어있는 @builder와 @AllArgsConstructor를 제거하고 아래 코드와 같이 id를 제외한 필드만 세팅하는 생성자에 @builder를 붙여주는게 좋을 것 같아요!

@Entity
@EntityListeners(AuditingEntityListener.class)
@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class Region {

    @Id
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    @Column(nullable = false)
    private Long id;

    @Column(nullable = false, length = 20, unique = true)
    private String name;

    @CreatedDate
    @Column(nullable = false, columnDefinition = "DATETIME(6)")
    private LocalDateTime createdAt;

    @LastModifiedDate
    @Column(nullable = false, columnDefinition = "DATETIME(6)")
    private LocalDateTime updatedAt;

    @OneToMany(mappedBy = "region", cascade = CascadeType.ALL, orphanRemoval = true)
    @Builder.Default
    private List<Store> storeList = new ArrayList<>();

    @Builder
    public Region(String name) {
        this.name = name;
    }

    public void addStore(Store store) {
        storeList.add(store);
        store.setRegion(this);
    }

    public void removeStore(Store store) {
        storeList.remove(store);
        store.setRegion(null);
    }
}

다른 클래스들도 이런식으로 수정해주시면 더 견고한 설계가 될 것 같습니다!

그리고 추가로 Lombok 사용법들에 관하여 구글링하면 잘 정리되어있는 글들이 많이 나오는데, 참고해보시면 좋을 것 같아요!
레퍼런스 하나 첨부하겠습니다!

https://velog.io/@wonizizi99/Spring-%EC%98%AC%EB%B0%94%EB%A5%B8-Lombok-%EC%82%AC%EC%9A%A9%EB%B2%95DtotoEntity-Builder

[strongmhk]

일대다의 관계 필드에 @Builder.Default 애노테이션을 사용하신 이유가 있을까요?

[strongmhk]

FoodQueryService에 Food 엔티티를 PK로 조회하는 메서드를 만들어서 foodQueryService.findByFoodId(storeRequestDTO.getRoodId())와 같이 참조하는 건 어떨까요? 그 방법이 재사용성 측면에서나, 계층간 참조의 간결성 면에서나 더 좋을 것 같아요!
그러면 각 도메인의 비즈니스 로직 계층에서만 영속성 계층을 참조하는 깔끔한 아키텍처가 만들어질 것 같습니다!
다른 도메인들도 그렇게 수정해보면 좋을 것 같아요!