Skip to content

Conversation

@beekeeper24
Copy link
Collaborator

서비스 단순 조회 로직에 readonly 어노테이션 추가
message,room controller,service 카멜케이스 수정
message servcie 사용하지 않는 로직 삭제

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/domain/userChat/chatmessage/controller/ChatMessageController.kt

🟢 좋은점:

  • ApiResponse 사용: GET과 POST API 모두 ResponseEntity<ApiResponse>로 응답을 감싸서 표준화되어 있으며, 성공 메시지("메시지 조회", "메시지 전송")를 포함해 일관된 응답 형식을 유지하고 있습니다.
  • Kotlin 최적화: null safety가 적절히 적용되어 있으며 (after == null 체크), 서비스 호출 부분에서 간결하게 처리되고 있습니다.
  • ktlint 규칙: 변수 이름 변경(msgSvc → messageService)으로 네이밍 컨벤션이 개선되었습니다. camelCase를 사용해 가독성이 높아졌고, 포맷팅(들여쓰기, 줄바꿈)도 diff 기준으로 문제없어 보입니다.

🟡 개선사항:

  • Kotlin 최적화: GET 매핑의 if-else 로직을 when 표현식으로 리팩토링하면 더 Kotlin다운 코드가 될 수 있습니다. 예: val messages = when { after == null -> messageService.getlistbefore(roomId, limit); else -> messageService.getlistafter(roomId, after) }. 이는 null safety를 강조하면서 코드 중복을 줄입니다.
  • ApiResponse 사용: Any 타입 대신 구체적인 제네릭 타입(예: ApiResponse<List>)을 사용하면 타입 안정성이 높아집니다. 현재 Any는 유연하지만, 컴파일 타임 체크를 약화시킬 수 있습니다.
  • ktlint 규칙: 메서드 이름(getlistbefore, getlistafter)이 camelCase지만, 'list' 부분이 약간 비표준적일 수 있습니다. 'getMessagesBefore', 'getMessagesAfter'처럼 더 명확한 네이밍으로 변경 고려.

🔴 문제점:

  • 글로벌 익셉션 처리: 이 컨트롤러에 직접적인 예외 처리(@ExceptionHandler나 try-catch)가 없으며, @ControllerAdvice가 프로젝트 수준으로 적용되었다고 가정하지만, 서비스 호출 시 발생할 수 있는 예외(예: roomId 유효성 검사 실패)에 대한 표준 에러 응답이 보장되지 않습니다. @ControllerAdvice를 통해 글로벌하게 처리되도록 확인하거나, 필요 시 로컬 핸들링 추가.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/domain/userChat/chatmessage/service/ChatMessageService.kt

🟢 좋은점:

  • Kotlin 최적화 측면에서 SendMessageReqdata class로 정의하여 간결하고 불변성을 유지하며, null safety가 자연스럽게 적용됨 (예: req.senderId, req.content 접근 시 null 체크 불필요).
  • @Transactional(readOnly = true)getlistbeforegetlistafter에 추가하여 읽기 전용 트랜잭션으로 성능 최적화와 일관성 보장.
  • deleteByRoom 메서드 삭제로 불필요한 코드 제거, 서비스의 책임 범위를 명확히 함 (ktlint 네이밍 컨벤션 준수).
  • Repository 이름 변경 (msgRepositorymessageRepository)으로 네이밍 컨벤션이 일관되게 개선 (ktlint 규칙 준수).
  • send 메서드에서 roomRepository.findById(roomId).ifPresent { ... }를 사용해 Optional 처리로 null safety를 고려한 Kotlin다운 코드 작성.

🟡 개선사항:

  • 메서드 이름 (getlistbefore, getlistafter)이 camelCase이지만, 더 명확하고 Kotlin 컨벤션에 맞게 getMessagesBeforegetMessagesAfter로 변경 고려 (ktlint 네이밍 규칙 강화).
  • getlistbefore에서 limit 파라미터를 받지만 실제 쿼리에서 무시되고 하드코딩된 Top50으로 동작; 동적 limit 적용 (e.g., findTopByRoomIdOrderByIdDesc(roomId, limit))으로 유연성 높임.
  • asReversed() 사용은 간단하지만, 대량 데이터 시 성능 이슈 발생 가능; Repository 쿼리를 OrderByIdAsc로 변경해 불필요한 리버스 연산 피함 (Kotlin 최적화).
  • send 메서드의 ifPresent는 Java Optional 스타일; Kotlin의 ?.let { ... }나 elvis 연산자 (?:)로 더 idiomatic하게 리팩토링하여 코드 가독성 향상.
  • when 표현식이나 확장 함수를 활용할 기회 (e.g., 메시지 유효성 검사 로직에

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/domain/userChat/chatroom/controller/ChatRoomController.kt

🟢 좋은점:

  • 모든 API 응답이 ApiResponse로 일관되게 감싸여 있으며, 성공 시 표준적인 msg와 data 구조를 사용하고 있습니다. (예: startChat, deleteChat, get 메서드)
  • Kotlin data class(StartChatReq)를 사용해 요청 객체를 간결하게 정의한 점이 좋습니다. null safety 측면에서 기본적으로 안전합니다.
  • ktlint 규칙 준수: roomSvc → roomService로 네이밍 컨벤션을 개선( camelCase 일관성), 코드 포맷팅이 깔끔합니다.

🟡 개선사항:

  • Kotlin 최적화: startChat 메서드에서 .id!! 연산자를 사용하고 있지만, null safety를 강화하기 위해 Elvis 연산자(?:)나 let/also 확장 함수를 활용해 null 체크를 명시적으로 하는 것이 좋습니다. 예: val roomId = roomService.exceptOneToOneRoom(req.guideId, req.userId)?.id ?: throw IllegalArgumentException("Room not found")
  • ApiResponse 사용 시 msg 필드가 모든 메서드에서 일관되게 사용되지 않습니다. (startChat과 get은 msg와 data, deleteChat은 msg만) – 표준화 위해 모든 응답에 msg를 포함하거나, 빈 msg를 피하세요.
  • 메서드 이름 개선 제안: exceptOneToOneRoom은 "createOrGetOneToOneRoom"처럼 의도를 더 명확히 하는 이름으로 변경하면 가독성이 높아집니다. deleteByOwner도 "deleteChatRoomByOwner"로 더 구체적으로.

🔴 문제점:

  • 글로벌 익셉션 처리 미적용: 컨트롤러에 @ControllerAdvice나 try-catch가 없어, 서비스 레이어 예외(예: RoomNotFoundException)가 발생 시 표준 에러 응답(ApiResponse)이 아닌 스프링 기본 에러가 반환될 수 있습니다. @ControllerAdvice를 글로벌로 적용하거나, 각 메서드에 예외 핸들링을 추가하세요. (예: 서비스 호출 시 wrap with try-catch and return ApiResponse with error)

@beekeeper24 beekeeper24 merged commit 9a8dad3 into main Sep 29, 2025
1 check passed
@beekeeper24 beekeeper24 deleted the refactor/be/1 branch September 29, 2025 02:17
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.

3 participants