[MVC 구현 3단계] 오리(오현서) 미션 제출합니다. #543
Conversation
| @@ -39,14 +36,14 @@ public void init() { | |||
|
|
|||
| private HandlerMapping initHandlerMappings() { | |||
| HandlerMapping handlerMapping = new HandlerMappingComposite( | |||
| List.of(new ManualHandlerMapping(), new AnnotationHandlerMapping("com.techcourse.controller"))); | |||
| List.of(new AnnotationHandlerMapping("com.techcourse.controller"))); | |||
There was a problem hiding this comment.
mvc 패키지에 있는 DispatcherServlet에서 특정 패키지에 의존적이게 되었네요.
다른 방법은 없을까요?!
There was a problem hiding this comment.
전 지정한 패키지만 스캔하고 싶어서 이렇게 명시적으로 하는 방식을 더 선호했어요.
조금 변경에 취약하더라도(그럼에도, 바뀔 가능성이 크지 않은 부분이기에 리스크가 크지 않다고 느꼈어요.) 예상치 못한 패키지를 스캔 하는 것에 대한 리스크가 조금 두려웠어요.
There was a problem hiding this comment.
그렇군요.
저는 DispatcherServlet이 mvc 패키지 하위에 있는 이상 개발자가 관여하는 부분이 아니라 프레임워크 부분이라고 생각했고, 개발자가 직접 지정할 수 없다고 생각했어요!
| @@ -39,14 +36,14 @@ public void init() { | |||
|
|
|||
| private HandlerMapping initHandlerMappings() { | |||
| HandlerMapping handlerMapping = new HandlerMappingComposite( | |||
There was a problem hiding this comment.
이 부분은 앞서 오리가 예시로 들어주었던 부분과 달라서 코멘트 남겨보아요!
만약 핸들러 매핑이 하나만 있다면 해당 핸들러 매핑을 직접 대입해주는 것이 이해하기 쉬운 코드라고 하셨는데, HandlerMappingComposite에 넣은 이유가 무엇인가요?
| handlerMapping.initialize(); | ||
| return handlerMapping; | ||
| } | ||
|
|
||
| private HandlerAdapterFinder initHandlerAdapterFinder() { | ||
| return new HandlerAdapterFinder( | ||
| List.of(new ManualHandlerAdapter(), new AnnotationHandlerAdapter())); | ||
| List.of(new AnnotationHandlerAdapter())); | ||
| } | ||
|
|
There was a problem hiding this comment.
마땅히 남길만한 곳이 없어서 여기에 남겨요!
이번 단계를 구현하면서 고민했던 부분이 있어서요! 오리가 구현해주신 ExceptionResolver에서 예외처리를 해주기 위해 특정 jsp의 이름이 들어간 JspView를 생성하고 있는데요. 그렇다면 만약 app에 해당 이름을 가진 파일이 없다면 어떻게 되는것인가요? 구현해주신 DispatcherServlet을 사용하기 위해서는 무조건 404.jsp와 500.jsp를 구현해주는 것을 강제해야 할 것 같네요. 이렇게 되면 조금 빡빡한(?) 구조가 될 것 같아서 고민이네요!
There was a problem hiding this comment.
이 부분은 생각해보니 좀 마음에 들지 않는 부분이네요.
중간에 interface를 쓰는 방식으로 개선해보았어요.
| private static final Logger log = LoggerFactory.getLogger(LoginController.class); | ||
|
|
||
| @RequestMapping(value = "/login", method = RequestMethod.POST) | ||
| public ModelAndView login(final HttpServletRequest req, final HttpServletResponse res) { |
There was a problem hiding this comment.
기존의 반환타입을 변경하지 않고 String도 호환되게 할 수도 있을 것 같아요.
하지만 변경하라는 말은 절대 아닙니다!!
There was a problem hiding this comment.
String을 받든 Model, View 무엇을 받든 ModelAndView로 만들어주는 클래스가 있으면 확장성 있겠네요.
다만 지금은 ModelAndView로만 받는 단순한 방식도 복잡하지 않기 때문에 괜찮다고 느껴져요.
| public ModelAndView handle(Throwable ex) { | ||
| ExceptionHandler handler = handlers.stream() | ||
| .filter(it -> it.support(ex)) | ||
| .findFirst() | ||
| .orElseThrow(() -> new InternalException()); | ||
| return handler.handle(); |
There was a problem hiding this comment.
오 이렇게 처리해주셨군요 ㅎㅎ 너무 좋아요.
여기서 추가적으로 만약 개발자가 ExceptionHandler를 구현하지 않아서 아무것도 support하지 않을때, mvc 자체적으로 예외 화면을 보여주는 식으로 해볼수도 있겠네요!
| @@ -39,14 +36,14 @@ public void init() { | |||
|
|
|||
| private HandlerMapping initHandlerMappings() { | |||
| HandlerMapping handlerMapping = new HandlerMappingComposite( | |||
| List.of(new ManualHandlerMapping(), new AnnotationHandlerMapping("com.techcourse.controller"))); | |||
| List.of(new AnnotationHandlerMapping("com.techcourse.controller"))); | |||
There was a problem hiding this comment.
그렇군요.
저는 DispatcherServlet이 mvc 패키지 하위에 있는 이상 개발자가 관여하는 부분이 아니라 프레임워크 부분이라고 생각했고, 개발자가 직접 지정할 수 없다고 생각했어요!
안녕하세요 밀리 MVC 미션의 마지막 단계네요.
이번 PR에서는 요구사항을 충족 시키는 것에 중점을 뒀습니다.
그리고 큰 부분은 아니지만, 기존의 컨트롤러에선 LoginView(GET 요청)와 Login(POST 요청)이 분리되어 있고, 페이지 요청은 login/view 로 매핑되어 있었는데, 둘다 /login을 사용하고 http method로 구분하도록 변경했습니다. 이유는 그게 깔끔한거 같아서..