[MVC 구현하기 - 1단계] 에단(김석호) 미션 제출합니다. #426
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CFalws
requested changes
Sep 14, 2023
| GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS, TRACE | ||
| GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS, TRACE; | ||
|
|
||
| public static RequestMethod from(final String method) { |
There was a problem hiding this comment.
예외 메세지를 custom 하려고 이렇게 했습니다. 다만 모디 글을 보니 switch 문도 나쁘지 않을 것 같아서 다음단계에서 바꿔보려고 합니다!
| return handlerExecutions.get(targetKey); | ||
| } | ||
|
|
||
| return new IllegalArgumentException("처리 할 수 없는 요청 입니다."); |
There was a problem hiding this comment.
HandlerExecutions에서 해당 메서드와 uri가 없으면 예외를 반환하네요?? 와우.. Optional, Null을 사용하지 않고 해당 방식으로 구현하신 이유가 무엇일까요?
There was a problem hiding this comment.
처리할 수 없는 메서드면 예외를 반환하는게 좋다고 생각해서 이렇게 구현했습니다.
2단계 미션을 보니 여러 HandlerMapping이 등장하는데요. 이럴 경우에는 각각의 handlerMapping에서 예외처리가 분산된다는 문제점이 발생할 것 같습니다! 이런 문제는 2단계하면서 직접 겪어보고 null이나 Optional을 리턴하도록 변경해보겠습니다~
Comment on lines
21
to
23
| final Constructor<?> constructor = controller.getDeclaredConstructor(); | ||
| final Object instance = constructor.newInstance(); | ||
| return (ModelAndView) method.invoke(instance, request, response); |
There was a problem hiding this comment.
매번 같은 인스턴스인데 요청이 들어올 때마다 새로운 인스턴스를 생성하면 객체 생성 비용에서 단점이 있다고 생각합니다.
혹시 이렇게 구현하신 이유가 있으실까요?
There was a problem hiding this comment.
크게 고민하지 못했던것 같아요. 수정해서 반영하겠습니다
| @@ -21,9 +23,19 @@ public AnnotationHandlerMapping(final Object... basePackage) { | |||
|
|
|||
| public void initialize() { | |||
| log.info("Initialized AnnotationHandlerMapping!"); | |||
| final Map<HandlerKey, HandlerExecution> handlerExecutions = Objects.requireNonNull(HandlerExecutionFactory.create(basePackage)); | |||
|
리뷰 반영해서 수정했습니다:) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요 에밀! 오랜만이네요. 미션 제출합니다!
아직은 Reflection을 어떻게 추상화할 지 몰라 class 분리만 했습니다.
이번 미션 잘 부탁드립니다🙇♂️🙇🙇♀️!
이번 PR 범위는 여기까지 입니다.