Step3-kakao-tech-campus / Team3_BE

[카테캠 1기] 번개 지향 볼링 모집 커뮤니티 "번개볼링"의 백엔드 서버입니다.
2 stars 4 forks source link

쪽지API 대화방목록조회 수정 #54

Closed yunzae closed 1 year ago

yunzae commented 1 year ago

Summary

대화방 목록 조회 수정

기존: 새로운메세지의 경우 유무만 알려줌 수정: 새로운메세지의 수까지 알려줌

Description

DTO 수정

레포지토리 메소드 추가

삭제된 레포지토리 메소드의 테스트 코드 삭제

+) 일대일대화방의 메소드 명을 변경 getMessagesByOpponent() -> getMessagesAndUpdateToRead()

Related Issue

Issue Number: close #50 close #44

yunzae commented 1 year ago

넵! 전체적으로 괜찮은 것 같습니다 👍 하나 궁금한 것이 있는데요.

public ResponseEntity<?>  // 1.
public ResponseEntity<ApiUtils.Response> // 2.

위의 코드에서 1번에서 2번으로 변경하셨던데, 이유가 있으신가요? 다른 코드(다른 controller 코드)들에서는 전부 1번 방식으로 Response를 반환해서 이점이 궁금해서 여쭤봅니다! 🤔

앗.. 일단 다시 원래대로 고쳐놨습니다.. 저희 코드 구조상 제네릭으로 둘 필요가 없어 보여서 혼자서 테스트하다가 그대로 올려버렸네욤..

yunzae commented 1 year ago

음.. 한가지 유지보수적인 측면에서 보자면... 🤔

service code에서

if (userId.equals(opponentId)){throw new Exception400("본인과 쪽지 대화를 할 수 없습니다.");} // 1

userRepository.findById(userId).orElseThrow(()->new Exception404("존재하지 않는 유저입니다.")); // 2
userRepository.findById(opponentId).orElseThrow(()->new Exception404("존재하지 않는 유저입니다.")); //3

1번 코드는 3번이나 중복이 되는 곳이 있고, 2, 3번은 주는 id값 (userId, opponentId) 만 다른상태에서 같은 함수를 호출해서 사용하고 있는데,

1번을 따로 빼서 userCheck 함수를 만드시고(추후에 exception 리팩토링 시 유용할 것 같음), 2,3번도 함수화 하셔서 parameter 값으로 id 값만 다르게 주시는 건 어떨까 싶습니다!

좋은 생각인 것 같습니다! 그런데 코드의 양이 너무 적어 추가적인 메소드를 만들면 오히려 복잡성을 높혀 가독성이 떨어질 것 같기도 해서 약간 고민이 되는 부분인 것 같습니다. 다른 분들은 어떻게 생각하시는 지 궁금합니다.

jagaldol commented 1 year ago

음.. 한가지 유지보수적인 측면에서 보자면... 🤔 service code에서

if (userId.equals(opponentId)){throw new Exception400("본인과 쪽지 대화를 할 수 없습니다.");} // 1

userRepository.findById(userId).orElseThrow(()->new Exception404("존재하지 않는 유저입니다.")); // 2
userRepository.findById(opponentId).orElseThrow(()->new Exception404("존재하지 않는 유저입니다.")); //3

1번 코드는 3번이나 중복이 되는 곳이 있고, 2, 3번은 주는 id값 (userId, opponentId) 만 다른상태에서 같은 함수를 호출해서 사용하고 있는데, 1번을 따로 빼서 userCheck 함수를 만드시고(추후에 exception 리팩토링 시 유용할 것 같음), 2,3번도 함수화 하셔서 parameter 값으로 id 값만 다르게 주시는 건 어떨까 싶습니다!

좋은 생각인 것 같습니다! 그런데 코드의 양이 너무 적어 추가적인 메소드를 만들면 오히려 복잡성을 높혀 가독성이 떨어질 것 같기도 해서 약간 고민이 되는 부분인 것 같습니다. 다른 분들은 어떻게 생각하시는 지 궁금합니다.

그렇더라도 메서드로 추출하게 되면 각 동작의 Exception과 Exception 메시지를 한 군데에서 관리할 수 있게 되어 에러메시지 관리가 쉬워지는 장점이 존재할 수 도 있을 거 같습니다.

1줄짜리라서 뭐가 낫다고 말하기 애매한거 같습니다

yunzae commented 1 year ago

userCheck 메소드와 getUser 메소드를 추가하여 중복된 코드를 제거하였습니다.