[박태식] sprint3

[martinP-Ghub]

요구사항

기본

• 기본 항목 1
• 기본 항목 2

심화

• 심화 항목 1
• 심화 항목 2

주요 변경사항

스크린샷

멘토에게

• 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

[JJong0416]

태식님 안녕하세요~! 🙇
이번 스프린트에는 저번에 리뷰 드린 내용들을 잘 반영해주신 모습이 인상적이였습니다 👍 저번 코드리뷰 드린 내용도 잘 적용시킨 것 같아요.

조금 더 개선해야 할 점은 객체지향적으로 코드를 작성하는 부분이 아직 미숙하신 것 같아요. 코드를 작성하실 때 어떻게 하면 객체지향적으로 작성할 수 있을지 고민해보시는 걸 추천드립니다.

이번 스프린트 너무너무 고생 많으셨고, 혹시 코멘트 관련하여 궁금하거나 이해가 안되는 점은 멘토링때 들고오시면 언제든 답변드리도록 하겠습니다 :D

[JJong0416]

개인적인 취향 차이일 수도 있는데요. 이렇게 enum 타입을 따로 클래스로 빼서 관리하는 팀들도 있지만, 타입에 대해서 도메인으로 묶어버리시는 분들도 많아요

개인적으로는 단순히 code를 관리하는 목적이라고 한다면 Channel 도메인에 넣어두는게 좋을 것 같아요 :D

클래스 내부에서 선언하는 방법도 알아두시면 좋을 것 같아요
향로님 enum 블로그

[JJong0416]

태식님이 알고 계신지 모르기 때문에 가상의 상황을 제공해볼게요

요구사항으로 만약에 A라는 유저 상태를 을 하나 만들었고, 요구사항중에 유저 상태 생성일로부터 1시간 후의 시간을 노출해야한다고 해봅시다.

UserStatus userStatus = new UserStatus(..., Instant.now());

// getter로 받은 lastSeenAt에 1시간 추가한 Instant를 생성
Instant requiredInstant= userStatus.getLastSeenAt().plusSeconds(3600);

System.out.println(userStatus.getCreatedAt());
System.out.println(requiredInstant);

각각 어떤 값이 나오는지 생각해보시는게 좋을 것 같아요!
왜 나왔는지 고민해보시고 아래 레퍼런스를 참고해보시길 바랍니다 :D
(실무에서 정말로 필요한 상황이 아니라면 Instant, Date는 잘 사용하지 않습니다)

effective-java item50 적시에 방어적 복사본을 만들어라

[JJong0416]

이런건 주석이나 _처리 하는 것 보단 새로 만드신 클래스 이름을 DiscodeitV2Application 으로 이름 바꿔주시면 file changed에 안잡히기 때문에 멘토 입장에서는 코드리뷰 할 때 조금 더 편합니다 :D
다음에 참고해서 해주시면 좋을 것 같아요!

[JJong0416]

👍 저번 리뷰를 잘 반영해주신 것 같아요.

[JJong0416]

Record의 히스토리를 한번 공부해보시는 걸 추천합니다.
예전에는 record를 Request DTO로 사용할 때는 상당히 불편한(?) 문제를 마주칠 수 있습니다.

• Jackson 역직렬화 문제 (Deserialization Issue)

Spring에서는 JSON 요청을 객체로 변환할 때 Jackson을 사용해 역직렬화를 수행하는데요. record에서는 기본적으로 기본 생성자가 따로 없습니다. 그래서 아래와 같이 작성해주셔야 합니다.

public record UserRequest(
        @JsonProperty("username") String username,
        @JsonProperty("password") String password,
        @JsonProperty("email") String email,
        @JsonProperty("phoneNumber") String phoneNumber,
        @JsonProperty("profileImageId") UUID profileImageId
//        @JsonProperty("profileImageName") String profileImageName
) {
    // @JsonCreator를 추가하여 명시적으로 생성자를 지정
    @JsonCreator
    public UserRequest(
            @JsonProperty("username") String username,
            @JsonProperty("password") String password,
            @JsonProperty("email") String email,
            @JsonProperty("phoneNumber") String phoneNumber,
            @JsonProperty("profileImageId") UUID profileImageId
            // @JsonProperty("profileImageName") String profileImageName
    ) {
        this.username = username;
        this.password = password;
        this.email = email;
        this.phoneNumber = phoneNumber;
        this.profileImageId = profileImageId;
        // this.profileImageName = profileImageName; // 주석처리된 부분도 필요에 맞게 추가 가능
    }
}

뭔가 상당히 불편해 보이시지 않나요 ^-^..?

@JsonCreator를 통해서 생성자를 명시적으로 지정하여 Jackson이 record 객체를 생성할 때 사용할 생성자를 알 수 있도록 해야 하고, @JsonProperty를 사용해서 JSON 필드 이름과 record 필드명을 매핑시켜줘야 역직렬화가 이루어지게 됩니다.

이러한 이유들 때문에 예전에는 안쓰는 팀도 많았습니다. 단순히 class에 Lombok 어노테이션을 쓰면 간소화 할 수 있기 때문이지요.

그런데 JDK 14부터는 그런 문제들이 해결되었다고 하더라구요. 그렇기 때문에 Record 도입은 버전을 잘 고려하시면서 적용시키면 좋을 것 같습니다

Jackson 2.12
Jackson databind

(나중에 프로젝트를 하시고, 면접에 가셨을 때 DTO를 record를 쓰신 이유를 물어보거나 역직렬화 이슈에 대해 면접관들이 물어보면, 잘 대답해주시면 면접관들이 좋아 죽습니다 😄)

[JJong0416]

특별한 이유가 아니라면 축약어는 지양해주시면 좋을 것 같아요 👍

[JJong0416]

아무리 1줄이더라도 {}를 넣어주세요. {}을 넣어줘야 다른 개발자가 추가적인 코드를 넣을 때 실수를 줄여줍니다.

클린 코드 - 가독성 좋은 코드

[JJong0416]

하나의 생성자로 모든 생성자를 대체하시려고 하는 것 같아요.
의미있는 생성자를 여러개 두는 것은 어떨까요?

또한 파라미터로 null을 넣는 부분이 많는 것 같아요. 태식님이 혼자 하는 프로젝트라면 문제가 없겠지만, 실무에 가시면 어떤 값을 null이 들어가는 것인지, 그리고 그 값이 비지니스적으로 문제가 없는 것인지 확인하기 어렵습니다.

만약 네이밍을 통해서 의미를 전달하고 싶다면 정적 팩토리 메소드를 고려해주시길 바랍니다 :D
effective-java 정적 팩토리 메소드를 사용하라

[JJong0416]

만약에 channelType이 하나 더 추가되면 if - else if - else, 또 하나가 추가되면 또 추가가 될 것 같아요.
결국에는 서비스 계층에서든, 아니면 엔티티에서든 분기는 하는 게 맞는데요. 어디서 하는지는 취향 차이일 것 같습니다 :D
ChannelType에 따라서 Enum에서 처리하는 방식도 있으니 공부해보시는 걸 추천합니다.

public enum ChannelType {
    PRIVATE {
        @Override
        public Channel createChannel(ChannelRequest request) {
            return Channel.createPrivate(request.member(), request.owner(), this);
        }
    },
    PUBLIC {
        @Override
        public Channel createChannel(ChannelRequest request) {
            return Channel.createPublic(request.name(), request.description(), request.owner(), this);
        }
    };

    public abstract Channel createChannel(ChannelRequest request);
}

위와 같이 하면 서비스 계층이 더 직관적으로 되지 않을까요?
(참고로 수도 코드로 DTO를 엔티티까지 들고오는 것은 적절하지 않기 때문에 저거 풀어 해쳐서 파라미터로 받는게 좋을 것 같아요 :D)

    @Override
    public ChannelResponse create(ChannelRequest request) {
        Channel channel = request.getChannelType().createChannel(request);
        channelRepository.save(channel);
        return ChannelResponse.fromEntity(channel);
    }

[JJong0416]

이렇게 되면 똑같이 특정 조건에 따라서 생성자가 서비스 계층에 있을 것 같아요.
도메인(Entity)가 무거워지는 것을 걱정하시는 것 같아요. 하지만 그게 객체지향의 추구점이기 때문에 고민해보시는 걸 추천합니다.