-
Notifications
You must be signed in to change notification settings - Fork 0
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] 신고 시 욕설 확인 기능 구현 #228
Conversation
@@ -16,29 +14,45 @@ | |||
@RequiredArgsConstructor | |||
public class ReportEventHandler { | |||
|
|||
private final ReportCheckClient reportCheckClient; | |||
private final ReportProcessor reportProcessor; |
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.
어차피 나중에 다시 람다 서버를 쓰게 되면서 기존 FeignClient
를 쓰는 ReportCheckClient
로 변경할 예정인거지?
그거와 별개로 ReportEventHandler
의 내부 코드를 전혀 건드리지 않으면서 이번 PR을 마무리하는 방법을 썼으면 더 좋았을 것 같은데 이건 내가 ReportCheckClient 인터페이스 설계를 잘못해서 애초에 무리였군 (반환값이 ResponseEntity라..)
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.
맞아!
ReportCheckClient 인터페이스 설계와는 별개로 ReportEventHandler 의 내부 코드를 건드리지 않고 신고 기능을 구현할 수 있었던 걸까?! 나는 건드리지 않는 방법이 떠오르지 않는데 어떤 방식을 생각했던 건지 물어봐도 될까?
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.
ReportCheckClient
인터페이스가 지금은 HTTP 요청을 한다는 거를 ResponseEntity
를 반환하고 있어서 지금은 코드 변경이 필연적일 수밖에 없어.
그런데 만약 인터페이스를 HTTP 의존적이지 않게 짰다면 ReportCheckClient
를 ReportProcesor
가 구현하게 하면 핸들러는 ReportCheckClient
를 의존한 코드 그대로 사용할 수 있었겠지.
그래서 이번참에 신고 처리 인터페이스를 제네럴하게 구현해서 ReportEventHandler
가 걔를 의존하게 하고, 그 인터페이스 구현체를 '람다 서버 이용하는 구현체', '내부 파일 이용하는 구현체'로 다르게 구현할 수 있을 것 같은데 리팩터링해보면 어떨까?
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.
아하 그렇겠다 무슨 말인지 이해했어!
이거까지 적용해서 수정해볼게
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.
추후 리팩토링 하기!
🌍 이슈 번호
📝 구현 내용
🍀 확인해야 할 부분