[Sku-Deview] 팀 프로젝트 피드백

 

들어가기 앞서..

 

[Sku-Deview] 개발자 커뮤니티 팀 프로젝트 회고

[Sku-Deview] 개발자 커뮤니티 팀 프로젝트 회고

 

나는 올해에 개발자 커뮤니티의 주제로 2인 프로젝트를 진행했었다. 해당 프로젝트는 멘토분같이 이끌어주는 사람이 없었기에 해당 프로젝트가 제대로 개발이 된것인지 확인을 받고 싶었다. 개발경험이 많고 피드백을 줄 수 있을만한 동기에게 프로젝트를 보여주어 피드백을 받을 수 있었다. 

이번에는 받았던 피드백들은 무엇이 있었는지와, 어떤 피드백을 수용할 것인지, 수용한 피드백에 대한 해결책은 어떤게 있을지 알아보겠다.

 

---

 

피드백 사항

프로젝트에 대한 피드백들은 다음과 같다.

1. 자세하거나 명확하지 않은 주석의 작성은 자원의 낭비를 초래할 수 있다.

내가 작성해주었던 주석의 예시를 보면 아래와 같다.

    /**
     * Create Member API
     *
     * @param create
     * @return ResponseStatus.SUCCESS_CREATE + Void
     */
    @PostMapping("/member")
    public ResponseFormat<Void> createMember(@RequestBody @Valid MemberRequestDto.CREATE create) {
        memberService.createMember(create);
        return ResponseFormat.success(ResponseStatus.SUCCESS_CREATE);
    }

주석은 의도와 의미를 명확하게 밝힐 수 있게 작성이 되어야한다. 하지만 내가 작성한 주석은 굳이 주석을 보지 않더라도, 코드에 이미 다 쓰여져 있는 내용일 뿐이다. 주석을 보아서 추가적으로 얻을 수 있는 정보들은 존재하지 않았다.. 

왜 이렇게 작성했을까?

주석을 작성하면 좋다는 이유를 자주 들었고, 팀 프로젝트를 진행하게 되면 주석을 작성 해줘야겠다는 압박감이 있었던 것 같다. 그렇기에 별다른 이유없이 주석을 작성했고, 그 결과 정말 의미없는 주석이 만들어지게 된 것 같다.
(남들이 다 한다는 어쭙잖은 이유로 애매하게 적용하려 해서 발생한 문제..)

 

 

2. @Valid, @Validated, @NotNull, @NotBlank 를 명확하게 사용해주자.

    @Column(name = "email", unique = true)
    @NotNull
    private String email;

정리1 | 정리2

상단에 정리를 잘 해놓은 글들을 참고했다. 

위의 정리한 글들을 보면서, 유효성 검증의 경우 최대한 controller에서 확인하고 service로 넘겨줘야 한다는 사실도 알게 되었다. 또한 항상 제대로 알아보지 않았던 NotNull, NotEmpty, NotBlank의 차이에 대해서도 알게 되었다.

• @NotNull: 해당 값이 null이 아닌지 검증함
• @NotEmpty: 해당 값이 null이 아니고, 빈 스트링("") 아닌지 검증함(" "은 허용됨)
• @NotBlank: 해당 값이 null이 아니고, 공백(""과 " " 모두 포함)이 아닌지 검증함

@Valid와 @Validated의 차이를 잘 몰랐었다. 오류가 안나고 PostMan이나 화면 구현 시 오류 메세지가 잘 출력이 된다면 '잘 됐나보다' 하는 막연한 생각을 갖고 구현을 했다고 착각했었다. 하지만 이번을 계기로 둘의 차이를 알게 되었고, @Valid를 사용하여 Controller에서 최대한 유효성 검사를 해주고, 유효성 검사가 필요한 속성에 @NotNull, @NotEmpty, @NotBlank 등 상황에 맞는 어노테이션을 부여해줄 것이다.

 

3. ResponseFormat을 custom으로 해주는 이유는 무엇인가? 

나는 하단과 같은 방식으로 ResponseFormat을 직접 Custom하게 작성하여 반환해줬었다. 하지만 해당 클래스를 custom하게 작성하여 반환해준 이유에 대한 피드백을 듣게 되었다.

솔직하게 말하면 구글링과 코드 복붙의 폐해라고 할 수 있다. 

초기에 pet-clinic 이라는 토이 프로젝트를 진행한적이 있었다. 해당 프로젝트를 진행하기 전에는 postman으로만 통신을 주고받았었고, controller에서의 반환해주는 ResponseEntity나 data만 던져주는 방식에 대해 전혀 알지 못했었다. 토이 프로젝트를 진행하며 다른 스터디원이 사용한 코드를 보게 되었고, 해당 방식이 정답이겠지라고 생각하고 무턱대고 복사하여 사용을 했었다. 

 

    @PostMapping("/login")
    public ResponseFormat<MemberResponseDto.READ> loginMember(@RequestBody @Valid MemberRequestDto.LOGIN login) {
        return ResponseFormat.successWithData(ResponseStatus.SUCCESS_OK,memberService.loginMember(login));
    }

@Getter
@AllArgsConstructor
@NoArgsConstructor
@Builder
public class ResponseFormat<T> {

    private boolean isSuccessful;

    private Optional<T> data;

    private String message;

    private HttpStatus statusCode;

    // Success - If the ResponseStatus is declared + return Only Message
    public static <T> ResponseFormat<T> success(ResponseStatus responseStatus) {
        return ResponseFormat.<T>builder()
                .isSuccessful(true)
                .data(Optional.empty())
                .message(responseStatus.getMessage())
                .statusCode(responseStatus.getStatusCode())
                .build();
    }

    // Success - If the ResponseStatus is Not declared + return Only Message
    public static <T> ResponseFormat<T> success(String message,
                                                HttpStatus httpStatus) {
        return ResponseFormat.<T>builder()
                .isSuccessful(true)
                .data(Optional.empty())
                .message(message)
                .statusCode(httpStatus)
                .build();
    }

    // Success - If the ResponseStatus is declared + return Message And Data
    public static <T> ResponseFormat<T> successWithData(ResponseStatus responseStatus,
                                                        T data) {
        return ResponseFormat.<T>builder()
                .isSuccessful(true)
                .data(Optional.ofNullable(data))
                .message(responseStatus.getMessage())
                .statusCode(responseStatus.getStatusCode())
                .build();
    }

    // Success - If the ResponseStatus is Not declared + return Message And Data
    public static <T> ResponseFormat<T> successWithData(String message,
                                                        HttpStatus httpStatus,
                                                        T data) {
        return ResponseFormat.<T>builder()
                .isSuccessful(true)
                .data(Optional.ofNullable(data))
                .message(message)
                .statusCode(httpStatus)
                .build();
    }

    // Failed - If the ResponseStatus is declared
    public static <T> ResponseFormat<T> error(ResponseStatus responseStatus) {
        return ResponseFormat.<T>builder()
                .isSuccessful(false)
                .data(Optional.empty())
                .message(responseStatus.getMessage())
                .statusCode(responseStatus.getStatusCode())
                .build();
    }

    // Failed - If the ResponseStatus is Not declared
    public static <T> ResponseFormat<T> error(String message,
                                              HttpStatus httpStatus) {
        return ResponseFormat.<T>builder()
                .isSuccessful(false)
                .data(Optional.empty())
                .message(message)
                .statusCode(httpStatus)
                .build();
    }

}

 

ResponseEntity와 해당 custom한 ResponseFormat은 크게 차이가 없다. 둘 다 모두 data와 statusCode를 보낼 수 있다. ResponseFormat에서 isSuccessful은 굳이 사용해줄 필요가 없는 속성이다. message의 경우라면 ResponseEntity에서도 보내줄 수 있지만 ResponseFormat에서 속성을 선언하여 사용해주는 방식이 더 편하긴 하다. 장단점은 분명히 존재한다. 

하지만 내가 받았던 질문은 '해당 기능 custom해서 사용해준 명확한 이유가 있어?',  '프론트와의 소통을 위해서인데 정말 그것을 위해 사용해 줄만한 속성을 담았어?',  '제공해주는 ResponseEntity를 사용해주거나, 아니면 정말 data만 던져주면 되지 않았어?'

만약 내가 정말로 custom을 사용해준 명확한 이유를 알았더라면, 내가 직접 써주겠다고 마음먹고 사용해준 코드라면 해당 질문에 대해 대답할 수 있었을 것이다. 하지만 코드에 대한 이해도 없이 사용했었기에 대답할 수 없었다.

해당 질문에 대해 하나씩 답변하자면, 다음과 같다. custom해서 사용해준 이유는 메서드를 구분하여 선언하여 성공시에 데이터와 함께 넘겨줄지 성공 코드만 넘겨줄지 정할 수 있고, message의 경우도 ResponseEntity를 사용해줄 때 보다 더 간단하게 넘겨줄 수 있다.

프론트와의 소통을 위해서인데 그것을 위해 사용해 줄만한 속성을 담았어? 에 대한 질문에는 뭐라고 답변할지 애매하다.. 아직 전문 프론트 개발자를 만나본적도 없고, 백엔드 개발자들끼리 모여 협업을 진행해본 경험이 전부여서 어떨 때 어떤 데이터를 넘겨 주는 것이 좋은지에 대해서는 더 고민해봐야 할 것 같다.

 

 

4. toList(), Collectors.toList() 의 사용 차이

해당 기능의 차이에 대해서는 생각해본적이 없었다. Collections.toList()만을 사용해주었지만 toList()도 사용이 가능하다는 것을 알게 되었다. 구글링을 통해 찾아보니, java버전이 오르면서 Strem.toList()의 사용이 권고 된다는 사실을 알게 되었다.

해당 Strem.toList()의 사용을 권고하는 이유는 다음과 같다.

java8에서 .collect(Collectors.toList()) 를 사용할 수 있는데, 문제는 리턴되는 List가 수정이 가능하기 때문에 java10 에서 수정불가능한(unmodifiable) List 로 반환되도록 toUnmodifiableList() 가 새롭게 등장했다고 한다. 하지만 toUnmodfiableList() 는 이름이 장황해서, java16 에서는 이를 보완하기 위해 Stream.toList() 가 등장했다고 한다.

쉽게 말해서 toList()는 불변하고, Collectors.toList()는 불변하지 않다는 것이다. 조금 더 자세하게 알아보면 다음과 같다.

• collect(toList()) : 수정 허용, Null 값 허용
• collect(toUnmodifiableList()): 수정 불가능, Null 값 비허용
• toList(): 수정 불가능, Null 값 허용

해당 기능을 잘 생각하여 앞으로 코드 작성 시, 불변이 필요한 List객체는 toList()로 사용해줘야겠다.

 

5. 명명 규칙?

명명 규칙에 대한 피드백도 듣게 되었다. 나름 잘 지킨다고 생각했는데, 그렇지 않았나보다. 피드백을 해준 사항은 다음과 같다.

BaseEntity에서 deleteAt이라고 사용해줬는데, boolean이고 삭제 여부를 저장하는 속성이라면 isDelete 가 맞지 않나? 라는 의견이 있었다. 다시 생각해보니 맞는 말이다. at은 시간에 관한 속성에 사용해주는 것인데 boolean 이고 여부를 저장하는 속성이라면 is를 붙여주는 명칭이 더 맞는 선택이다.

@MappedSuperclass
@Getter
@EntityListeners(AuditingEntityListener.class)
public abstract class BaseEntity {

    @CreatedDate
    @Column(name = "reg_date", nullable = false, updatable = false)
    private LocalDateTime regDate;

    @LastModifiedDate
    @Column(name = "mod_date", nullable = false)
    private LocalDateTime modDate;

    // true  = 삭제된 상태 , false = 삭제되지 않은 상태
    @Column(name = "delete_at")
    private boolean deleteAt = false;

    public void changeDeleteAt() {
        this.deleteAt = true;
    }
}

 

또한 추가적으로 클래스명들이 전체적으로 너무 길다는 의견을 들었다. 예를 들어, MemberRequestDto, MemberResponseDto 같은 것들이나, 회원과 게시글 좋아요를 저장하는 MemberListPostRepository 등이 있었다. 

나같은 경우에는 클래스 명칭이 길어도 괜찮을 거라고 생각했었고, 또한 명확하게 클래스 명을 모두 적어주는 것이 좋다는 생각이 있었다. 하지만 클래스 명을 줄일 수 있고 줄여도 명확하게 구분이 가고 해당 클래스에 대해 알 수 있다면 줄이는 방식을 사용해주는게 좋을 것 같다는 생각이 들었다.

 

---

 

정리