[Web-Renderer] PostHandler 패키지 -> Renderer 패키지로 변경#32
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
리팩토링 작업이 전체적으로 잘 진행되었습니다. PostHandler → Renderer 패키지 변경과 네이밍 개선은 의도가 명확합니다.
다만 몇 가지 안정성과 유지보수성 관련 사항을 지적합니다:
- StaticViewResponse의 객체 불변성:
path필드가 가변이므로 예상치 못한 상태 변화가 발생할 수 있습니다. - 메서드 네이밍 일관성: AppConfig에서 메서드명과 반환 타입이 불일치하여 혼동을 야기합니다.
- 팩토리 메서드 설계: 세 개의
of()오버로드 간 용도가 불명확하여 API 사용성이 떨어집니다. - 렌더러 통합 확인: 두 개의 응답 핸들러를 하나로 병합했다고 했으나, 렌더러 목록 확인이 필요합니다.
이 항목들을 검토하고 개선하면 코드 품질이 한층 향상될 것입니다.
There was a problem hiding this comment.
path 필드를 final로 선언해야 합니다. 현재는 생성 후 setPath()로 변경 가능하므로 객체 불변성이 보장되지 않습니다. 만약 동적 설정이 필요하다면 생성자에서 완전히 초기화하거나, 빌더 패턴을 고려하세요.
There was a problem hiding this comment.
팩토리 메서드 3개의 용도가 불명확합니다. of(String path)와 of(HttpStatus status, String viewPath) 간의 차이가 명확하지 않으며, of(HttpStatus status) 만으로는 경로 없이 생성되어 getPath()가 빈 문자열을 반환합니다. 각 메서드의 목적을 명확히 하거나 불필요한 오버로드를 제거하세요.
| } | ||
| private ViewResponseHandler viewResponseHandler(){ | ||
| return new ViewResponseHandler(); | ||
| } | ||
| private StaticContentResponseHandler staticContentResponseHandler(){ | ||
| return new StaticContentResponseHandler(); | ||
| private StaticViewRenderer staticContentResponseHandler(){ | ||
| return new StaticViewRenderer(); | ||
| } |
There was a problem hiding this comment.
메서드명 staticContentResponseHandler()와 반환 타입 StaticViewRenderer가 불일치합니다. StaticViewRenderer를 반환하므로 메서드명을 staticViewRenderer()로 변경하세요. 혼동을 초래할 수 있습니다.
src/main/java/config/AppConfig.java
Outdated
There was a problem hiding this comment.
PR 설명에서 "StaticContentResponse와 ViewResponse 병합 => StaticViewResponse"라고 했는데, 렌더러 목록에 하나만 있습니다. 이전에는 ViewResponseHandler와 StaticContentResponseHandler 두 개가 있었으나, 현재는 StaticViewRenderer 하나만 등록되었습니다. 두 응답 타입이 실제로 통합되었다면 확인 필요하고, 아니라면 누락된 렌더러가 있는지 검토하세요.
| package web.renderer; | ||
|
|
||
| import http.response.HttpResponse; | ||
| import web.response.HandlerResponse; | ||
|
|
||
| public interface WebHandlerResponseRenderer { | ||
| boolean supports(HandlerResponse response); | ||
| HttpResponse handle(HandlerResponse response); |
There was a problem hiding this comment.
PR 리뷰 포인트에 대한 답변: WebHandlerResponseRenderer가 HttpResponseRenderer보다 적절합니다. 이유:
- 이 인터페이스는 "웹 핸들러의 응답 객체를 HTTP 응답으로 변환"하는 역할입니다.
HttpResponseRenderer는 "이미 HTTP 응답을 렌더링한다"는 의미로 보여 역할이 불명확합니다.WebHandlerResponse(지금의HandlerResponse)를 처리한다는 점에서 현재 네이밍이 더 정확합니다.
1 participant
💻 작업 내용
✨ 리뷰 포인트
WebHandlerResponseRenderer의 네이밍이 적절한지 고민입니다.HttpResponse를 렌더링해준다는 관점에서
HttpResponseRenderer라 해야할지, 지금의 네이밍이 더 적절한지 의견주세요.구조는 특별히 바뀐게 없습니다.
🎯 관련 이슈
closed #31