-
Notifications
You must be signed in to change notification settings - Fork 1
[Refactor/#231] 종원 서버 1차 리팩토링 #237
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
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!!
PR 제목을 [{대문자키워드}/#{이슈번호}] 형태로 변경하면 좋을 것 같습니다~~
|
|
||
| ChatResponseDTO.SendMessageResponseDTO responseDTO = ChatConverter.toSendMessageDTO(saved); | ||
| simpMessagingTemplate.convertAndSend("/sub/chat/" + request.getRoomId(), responseDTO); | ||
| ChatResponseDTO.SendMessageResponseDTO responseDTO = ChatResponseDTO.SendMessageResponseDTO.toSendMessageDTO(saved); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
엔티티 -> DTO로 변환하는 메소드는 "from", "of"와 같이 네이밍 하는 것이 관레적이라 따르면 좋을 것 같습니다!!
from은 엔티티들로부터 생성되는 의미를 담고, of는 개별 필드들의 조합으로 만들어짐을 의미합니다.
DTO -> 엔티티일 때 주로 "toEntity" 혹은 "to엔티티이름"을 사용합니다.
| request.roomId(), | ||
| savedDTO.message(), | ||
| savedDTO.sentAt(), | ||
| totalUnreadCount); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
마지막에서 괄호를 내려주는 방식으로 코드를 통일하면 좋을 것 같습니다
ChatRoomUpdateDTO updateDTO = ChatRoomUpdateDTO.toChatRoomUpdateDTO(
request.roomId(),
savedDTO.message(),
savedDTO.sentAt(),
totalUnreadCount
);
- 이유는 다음과 같습니다.
- 인자 추가/삭제의 용이성: 새로운 인자를 추가하거나 마지막 인자를 지울 때, );가 붙어있으면 해당 줄까지 수정해야 하지만, 괄호가 내려가 있으면 중간 줄만 건드리면 됩니다. (특히 깃(Git) 변경 이력을 볼 때 한 줄만 깔끔하게 바뀌어 보입니다.)
- 가독성: 인자가 3~4개 이상으로 길어지는 경우, 아래처럼 괄호를 내려주는 것이 함수의 범위를 한눈에 파악하기 훨씬 쉽습니다.
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class RedisPublisher { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redis를 Publisher의 목적으로 다른 곳에서도 사용할 수 있으니 Chatting용의 느낌이 들어가도록 파일명을 수정하면 좋을 것 같습니다.
| messagingTemplate.convertAndSend("/sub/chat/" + roomId, messageDto); | ||
| } | ||
|
|
||
| public void handleUserUpdate(String channel, String jsonMessage) throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다른곳에서 사용하는게 아니라면 private로 처리하는 것이 좋아보입니다
| this.partner = partner; | ||
| } | ||
|
|
||
| public static ChattingRoom toCreateChattingRoom(Admin admin, Partner partner) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어짜피 static 메소드니까 이것도 from으로 하는게 좋을 것 같습니다!
| private Long opponentId; | ||
| } | ||
| public record BlockMemberRequestDTO( | ||
| Long opponentId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#️⃣연관된 이슈
📝작업 내용
🔎코드 설명(스크린샷(선택))
💬고민사항 및 리뷰 요구사항 (Optional)
비고 (Optional)