feat : ScheduledExecutorService -> RabbitMQ 전환 #97
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -40,6 +40,7 @@ dependencies { | |||
| compileOnly 'org.jetbrains:annotations:23.0.0' | |||
|
|
|||
| implementation "org.springframework.cloud:spring-cloud-starter-openfeign" | |||
| implementation 'org.springframework.boot:spring-boot-starter-amqp' | |||
|
|
|||
| implementation 'com.fasterxml.jackson.core:jackson-databind' | |||
| implementation 'io.jsonwebtoken:jjwt-api:0.11.5' | |||
There was a problem hiding this comment.
이 코드 패치를 간단하게 검토해보면, 버전 번호가 업데이트되었고 새로운 의존성인 'org.springframework.boot:spring-boot-starter-amqp'가 추가된 것 같습니다. 이 변경 사항은 특별한 문제가 없어 보입니다. 그러나 추가 된 새로운 의존성을 사용하여 코드의 다른 부분과 충돌하지 않는지 확인해야할 수도 있습니다. 이를 위해서는 코드를 전반적으로 검증하고, 작동 여부를 확인하는 프로세스가 필요할 것입니다.
| throw InternalServerJsonException.fail(e.getMessage()); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
위 코드는 RabbitMQ를 이용한 ReservationProducerAdapter 클래스입니다.
해당 코드에 대한 개선점은 크게 없어 보입니다. 다만, 문자열로 변환하기 위한 parseToJson 메소드의 이름을 toJson으로 변경하면 더 명확하고 일관성있게 될 것 같습니다.
또한, sendMessageToQueue 메소드에서 메시지를 생성할 때, MessagePropertiesBuilder는 여러 메시지 프로퍼티를 설정해줄 수 있으므로, 호출 시 매개변수로 넘기는 값을 final 변수에 담아 사용하는 습관을 들이는 것이 좋습니다.
코드리뷰에서 발견된 버그나 오류는 없습니다.
| @@ -4,6 +4,7 @@ | |||
| import java.util.Map; | |||
| import java.util.concurrent.ScheduledFuture; | |||
|
|
|||
| @Deprecated | |||
| public final class ReservationScheduledFutures { | |||
|
|
|||
| private final Map<Long, ScheduledFuture<?>> values = new HashMap<>(); | |||
There was a problem hiding this comment.
위 코드는 개인적으로 매우 작지만, 여전히 코드 리뷰를 할 수 있는 몇 가지 문제가 있습니다.
-
@Deprecated애너테이션은 클래스 전체에 적용되어 있습니다. 이것은 클래스가 더 이상 유용하지 않다는 것을 나타내지만, 그 이유가 설명되어 있지 않습니다.@Deprecated사용 이유와 함께 클래스 문서화를 추가하는 것이 좋을 것입니다. -
values변수는private로 선언되어 있지만,ReservationScheduledFutures클래스의 상태를 변경할 수 있으므로 이 변수에 대한 직접적인 접근이 가능합니다. 이러한 문제를 방지하기 위해values변수를 변경 불가능하도록 만드는 것이 좋습니다. -
Map인터페이스의 인스턴스가 생성되지 않았습니다 (new HashMap<>()). 초기화되지 않은 객체에서 NullPointException과 같은 다양한 예외가 발생할 수 있습니다.ReservationScheduledFutures클래스의 생성자에서 이들을 해결할 필요가 있습니다.
그 외에도, 해당 클래스를 사용하는 디바이스에서 Sun.misc.Unsafe 라이브러리의 기능을 차단하여 성능 향상을 얻을 수 있습니다. 추가 제안 사항이나 버그가 있으면, 지정해 주시기 바랍니다.
| @@ -11,6 +11,7 @@ | |||
| import java.util.concurrent.ScheduledFuture; | |||
| import java.util.concurrent.TimeUnit; | |||
|
|
|||
| @Deprecated | |||
| @Component | |||
| public final class ReservationSchedulerAdapter implements ReservationSchedulerPort { | |||
|
|
|||
There was a problem hiding this comment.
위 코드는 Java로 작성된 ReservationSchedulerAdapter 클래스 코드입니다.
버그와 개선 제안:
- @deprecated 애너테이션이 있는데, 이유가 없어 보입니다. 만약 클래스를 Deprecated하기로 결정했다면 그 이유를 명시하는 것이 좋습니다.
- 클래스 이름에서 Adapter 접미사로 인터페이스에 대한 어댑터 역할을 수행하는 것으로 보입니다. 하지만, 해당 인터페이스가 코드 중에서 참조되지 않았으며, 이 클래스를 사용하는 곳도 없어 보입니다. 따라서, 해당 인터페이스에 대한 사용처를 파악하고, 필요한 기능만 구현하는 것이 좋습니다.
코드 리뷰:
- import 문은 모두 정렬이 잘 되어 있습니다.
- 메소드 이름들이 충분히 명확합니다.
- 클래스 멤버 변수나 메소드의 가시성(modifier)을 지정하는데 일관성이 있습니다.
- 단어 선택도 적절히 되어 있습니다. (~Adapter, ~Port 등)
이 코드는 불완전해 보이지만, 필요한 경계 조건과 소스코드가 안보이므로 전체적인 코딩 흐름 파악이 어렵습니다. 위에서 언급해 드린 부분을 수정하여, 인터페이스에 대한 사용처를 파악한 후 필요한 부분만 구현함으로써, 코드를 더욱 개선할 수 있을 것입니다.
|
|
||
| @RabbitListener(queues = "reservation.queue") | ||
| void consumeMessageFromQueue(DelayReservationMessageRequestDto requestDto); | ||
| } |
There was a problem hiding this comment.
이 코드 조각에 대한 간략한 코드 리뷰를 도와드리겠습니다.
- 해당 인터페이스는 RabbitMQ에서 메시지를 소비하는 데 사용됩니다.
- 'backend.wal.reservation' 패키지에서 'DelayReservationMessageRequestDto' DTO 데이터 객체가 가져와지고 있습니다.
- @RabbitListener 주석을 통해 'reservation.queue' 큐에서 메시지를 수신하는 방법이 설정되어 있습니다.
- 이 코드 조각은 멀티 스레드 환경에서 안전하지 않으므로, 적절한 동기화 처리를 고려해야 할 수도 있습니다.
- 이 코드를 개선하기 위해서는 각각의 기능에 대한 테스트 코드 작성과 예외 처리를 추가하는 것이 좋을 것입니다.
관련 이슈
feat :#96
bug : #98