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

feat : 뿌리오 메시지 관련 로직 일부 #26

Closed
wants to merge 2 commits into from
Closed

Conversation

lsh1215
Copy link
Member

@lsh1215 lsh1215 commented Nov 4, 2024

뿌리오 개발 명세에 올라온 상태조회 샘플코드 일부 수정 및 빌드 확인(기능적으로 돌아가는 건 없음)

테스트 결과

ex) 베이스 브랜치에 포함되기 위한 코드는 모두 정상적으로 동작해야 합니다. 결과물에 대한 스크린샷, GIF, 혹은 라이브 데모가 가능하도록 샘플API를 첨부할 수도 있습니다.

+α Checklist

  • 자바 코드 컨벤션을 지키면서 프로그래밍했는가?
  • 한 메서드에 오직 한 단계의 들여쓰기(indent)만 허용했는가?
  • else 예약어를 쓰지 않았는가?
  • 3개 이상의 인스턴스 변수를 가진 클래스를 구현하지 않았는가? (가능하면 인스턴스 변수의 수를 줄이기 위해 노력한다.)
  • setter 없이 구현했는가?
  • 메소드의 인자 수를 제한했는가? (4개 이상의 인자는 허용하지 않는다. 3개도 가능하면 줄이기 위해 노력해 본다.)
  • 코드 한 줄에 점(.)을 하나만 허용했는가?
  • 메소드가 한가지 일만 담당하도록 구현했는가?
  • 클래스를 작게 유지하기 위해 노력했는가?

뿌리오 개발 명세에 올라온 상태조회 샘플코드 일부 수정 및 빌드 확인(기능적으로 돌아가는 건 없음)
@lsh1215 lsh1215 linked an issue Nov 4, 2024 that may be closed by this pull request
3 tasks
// messageService.requestSend(kkaoMessage);
// }

}
Copy link

Choose a reason for hiding this comment

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

코드에서 수정이 필요하거나 개선할 수 있는 부분은 다음과 같습니다.

  1. 메소드 분리: 현재 sendMessage 메소드가 commonMessage와 카카오 메시지 모두를 처리하려는 의도가 보입니다. 각 메시지 유형에 대한 메소드를 명확히 분리하는 것이 좋습니다. 주석 처리된 kkaoSendMessage 메소드를 복원하고, 두 메소드를 별도로 유지하세요.

  2. HTTP 응답 상태 코드: 현재 sendMessage 메소드는 HTTP 응답을 반환하지 않습니다. 성공적인 요청에는 ResponseEntity를 이용해 적절한 상태 코드(예: 201 Created)를 반환하는 것이 좋습니다.

  3. 예외 처리: 메시지 발송 중 오류가 발생할 경우 사용할 예외 처리가 없습니다. @ControllerAdvice 또는 특정 예외 핸들러를 통해 사용자 친화적인 에러 메시지와 적절한 상태 코드를 반환하도록 개선할 수 있습니다.

  4. DTO 클래스 패키지 구조: DTO 클래스 이름(MessageRequestDto.CommonMessage, MessageRequestDto.kkaoMessage)이 사용하기에 가독성이 떨어질 수 있습니다. 각 메시지 타입에 대해 별도의 DTO 클래스를 만드는 것을 검토해보세요.

  5. API 문서화: Swagger 등의 도구를 활용하여 API 문서화를 진행하면 클라이언트와의 통신 시 이해도를 높일 수 있습니다.

수정된 코드 예시는 다음과 같습니다:

@RestController
@RequiredArgsConstructor
@RequestMapping("/messages")
public class MessageController {

    private final MessageService messageService;

    // 메시지 발송
    @PostMapping("/common")
    public ResponseEntity<Void> sendCommonMessage(@RequestBody MessageRequestDto.CommonMessage commonMessage) {
        messageService.requestSend(commonMessage);
        return ResponseEntity.status(HttpStatus.CREATED).build();
    }

    // 카카오 메시지 발송
    @PostMapping("/kakao")
    public ResponseEntity<Void> sendKakaoMessage(@RequestBody MessageRequestDto.KakaoMessage kakaoMessage) {
        messageService.requestSend(kakaoMessage);
        return ResponseEntity.status(HttpStatus.CREATED).build();
    }
}

위와 같이 개선하여 코드의 가독성과 유지보수성을 높일 수 있습니다.

return authorization;
}

}
Copy link

Choose a reason for hiding this comment

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

코드는 전반적으로 잘 작성되어 있습니다. 하지만 몇 가지 개선 사항을 제안합니다.

  1. 불변성(Immutable): 객체의 상태를 변경할 수 없도록 하는 것이 좋습니다. Request 클래스의 필드를 final로 선언하여 불변성을 보장할 수 있습니다.

    private final String requestUri;
    private final String authorization;
  2. 공백 및 들여쓰기: Java에서는 일반적으로 클래스의 멤버 변수 선언과 메소드 간에 줄 바꿈을 사용하는 것이 좋으므로 가독성을 높일 수 있습니다.

  3. Override toString() 메소드: 객체를 로그하거나 디버그할 때 유용하도록 toString() 메소드를 오버라이드하는 것을 고려해보세요.

    @Override
    public String toString() {
        return "Request{" +
                "requestUri='" + requestUri + '\'' +
                ", authorization='" + authorization + '\'' +
                '}';
    }
  4. Null 체크: 생성자에서 null 값을 전달받는 경우를 처리할 수 있도록 예외 처리를 추가하면 더 안전합니다.

    public Request(String requestUri, String authorization) {
        if (requestUri == null || authorization == null) {
            throw new IllegalArgumentException("URI and authorization cannot be null");
        }
        this.requestUri = requestUri;
        this.authorization = authorization;
    }

이러한 점들을 반영하면 코드의 안정성과 가독성을 높일 수 있습니다.

private String senderNumber;
}

}
Copy link

Choose a reason for hiding this comment

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

코드 리뷰 결과, 몇 가지 수정 및 개선 사항을 제안드립니다.

  1. 클래스 이름의 일관성:

    • kkaoMessage 클래스는 Java의 클래스 네이밍 규칙에 따라 대문자로 시작해야 합니다. KakaoMessage와 같이 수정하는 것이 좋습니다.
  2. 중복 코드:

    • CommonMessageKakaoMessage에서 사용되는 일부 필드(예: messageType, content, targetCount)가 중복되어 있습니다. 이러한 공통 필드를 가진 부모 클래스를 만들어 상속받도록 하면 중복을 줄일 수 있습니다.
  3. Access Modifiers:

    • 모든 DTO 클래스에서 기본 생성자와 getter 외의 접근 제한자를 명시하지 않았습니다. 불필요한 인스턴스 생성을 방지하기 위해 이러한 클래스를 private 또는 protected로 만들고 필요한 경우 factory method를 사용하십시오.
  4. 타입 일관성:

    • targetCount 필드의 타입이 int에서 Long으로 바뀌어야 할 필요성이 있습니다. 이는 나중에 큰 숫자를 처리할 때 유용할 것입니다. KakaoMessageCommonMessage에서 일관성을 유지하세요.
  5. 오류 처리:

    • 각 DTO 필드에 대해 중요한 필수 항목이 있다면, 생성자에서 에러 처리를 추가하거나 유효성 검사를 수행하여 잘못된 데이터가 입력되지 않도록 할 수 있습니다.
  6. 명확한 주석 추가:

    • 각 DTO 클래스와 주요 필드에 대한 설명을 주석으로 추가하면 코드 읽기가 더 수월해질 것입니다.
  7. Lombok 사용 확인:

    • Lombok의 @Getter 관련 어노테이션은 필드가 private이면 정상작동하지만, 전체적인 사용 방식이 바람직한지 검토하고 필요 시 @Data 같은 다른 어노테이션을 사용해 getter/getter와 같은 오류 가능성을 줄일 수도 있습니다.

리팩토링을 통해 코드의 가독성을 높이고 유지보수를 쉽게 하기를 권장합니다.

package com.pictalk.message.dto;

public class MessageResponseDto {
}
Copy link

Choose a reason for hiding this comment

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

코드에서 다음과 같은 부분들을 개선할 수 있습니다:

  1. 패키지 구조: com.pictalk.message.dto 패키지가 의미가 있지만, DTO 클래스에 필요한 필드와 메소드를 추가하는 것이 좋습니다.

  2. 필드 추가: MessageResponseDto 클래스는 현재 비어있습니다. 보통 DTO에는 데이터 전송에 필요한 속성들이 포함되어야 합니다. 예를 들어, 메시지 ID, 내용, 발신자 ID 등을 필드로 추가할 수 있습니다.

  3. 생성자 및 Getter/Setter 방법 추가: 필드가 추가된다면, 생성자(constructor), getter 및 setter 메소드를 자동으로 생성하는 것도 좋은 습관입니다. Lombok 라이브러리를 사용할 수도 있습니다.

  4. 클래스 주석 추가: 코드의 가독성을 높이기 위해 클래스를 설명하는 주석을 추가하는 것도 좋습니다.

  5. Equals 및 HashCode 메소드 오버라이드: 비교 연산이 필요할 경우, Equals 및 HashCode 메소드를 오버라이드 하는 것이 좋습니다.

예시:

package com.pictalk.message.dto;

public class MessageResponseDto {
    private Long messageId;
    private String content;
    private Long senderId;

    // 생성자
    public MessageResponseDto(Long messageId, String content, Long senderId) {
        this.messageId = messageId;
        this.content = content;
        this.senderId = senderId;
    }

    // Getter 및 Setter
    public Long getMessageId() {
        return messageId;
    }

    public void setMessageId(Long messageId) {
        this.messageId = messageId;
    }

    public String getContent() {
        return content;
    }

    public void setContent(String content) {
        this.content = content;
    }

    public Long getSenderId() {
        return senderId;
    }

    public void setSenderId(Long senderId) {
        this.senderId = senderId;
    }
    
    // toString(), equals(), hashCode() 추가 가능
}

이렇게 하면 이 클래스의 활용도가 높아지고 유지보수하기 쉬운 코드가 됩니다.

key: ${PPURIO_API_KEY}
account: ${PPURIO_ACCOUNT}

Copy link

Choose a reason for hiding this comment

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

이 코드에서 몇 가지 개선점과 수정할 부분을 제안하겠습니다.

  1. 주석 추가: 코드의 각 섹션에 대한 간단한 주석을 추가하여 설정의 목적을 명확히 할 수 있습니다. 예를 들어, JWT와 ppurio의 설정에 대한 설명을 적어 두면 좋습니다.

  2. 환경 변수 검증: ${JWT_SECURITY_KEY}, ${PPURIO_API_URL}, ${PPURIO_API_KEY}, ${PPURIO_ACCOUNT} 같은 환경 변수가 올바르게 설정되었는지 검증하는 로직을 추가하면 런타임 오류를 방지할 수 있습니다.

  3. 일관성 유지: jwtppurio 설정을 통일감 있게 배열하는 것이 좋습니다. 예를 들어, 모든 보안 키 항목을 한 블록으로 모아서 정리할 수 있습니다.

  4. 포맷팅: 마지막 줄에 있는 "No newline at end of file" 경고는 불필요한 오류를 초래할 수 있으므로 파일 끝에 새로운 줄을 추가하는 것이 좋습니다.

  5. YAML 구조: YAML 형식에서 들여쓰기가 중요하므로, ppurio 섹션이 jwt 섹션과 잘 맞물려 있는지 확인하고 적절히 정렬해야 합니다.

이러한 간단한 조정을 통해 코드의 가독성과 유지보수성을 향상시킬 수 있습니다.

test resource의 application.yml 수정
@Enble Enble closed this Nov 20, 2024
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.

[feat] 문자 전송 api
2 participants