-
Notifications
You must be signed in to change notification settings - Fork 302
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
[MVC 구현하기 - 1단계] 디노(신종화) 미션 제출합니다. #382
Changes from 9 commits
52ee778
e7c21ab
3d8dd3b
0c6cf1c
8c9e0f0
7d577e4
0a58476
9a23e59
949a04e
35416b5
e817b82
1e50792
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package com.techcourse; | ||
|
||
import static org.junit.jupiter.api.Assertions.*; | ||
|
||
class DispatcherServletTest { | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,17 @@ | ||
package webmvc.org.springframework.web.servlet.mvc.tobe; | ||
|
||
import context.org.springframework.stereotype.Controller; | ||
import jakarta.servlet.http.HttpServletRequest; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import java.lang.reflect.Method; | ||
import java.util.Arrays; | ||
import java.util.HashMap; | ||
import java.util.Map; | ||
import java.util.Set; | ||
import org.reflections.Reflections; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import web.org.springframework.web.bind.annotation.RequestMapping; | ||
import web.org.springframework.web.bind.annotation.RequestMethod; | ||
|
||
public class AnnotationHandlerMapping { | ||
|
||
|
@@ -20,10 +26,30 @@ public AnnotationHandlerMapping(final Object... basePackage) { | |
} | ||
|
||
public void initialize() { | ||
final Reflections reflections = new Reflections(basePackage); | ||
final Set<Class<?>> controllerClasses = reflections.getTypesAnnotatedWith(Controller.class); | ||
|
||
controllerClasses.forEach( | ||
aClass -> Arrays.stream(aClass.getDeclaredMethods()).forEach( | ||
method -> { | ||
final RequestMapping requestMapping = method.getAnnotation(RequestMapping.class); | ||
if (requestMapping != null) { | ||
putHandlerExecutions(method, requestMapping); | ||
} | ||
} | ||
) | ||
); | ||
log.info("Initialized AnnotationHandlerMapping!"); | ||
} | ||
|
||
private void putHandlerExecutions(final Method method, final RequestMapping requestMapping) { | ||
Arrays.stream(requestMapping.method()) | ||
.map(requestMethod -> new HandlerKey(requestMapping.value(), requestMethod)) | ||
.forEach(handlerKey -> handlerExecutions.put(handlerKey, new HandlerExecution(method))); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 중요한 부분은 아니지만 stream 최종연산에서 stream 외부에 있는 객체의 접근은 최대한 피하는게 좋다고 합니다 ! (스트림은 함수형 프로그래밍을 위해 등장했기 때문에 비동기 동작시 문제가 될 수 있습니다 🤓) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 어렵네요.. 해당 부분은 다시 for문으로 바꾸어 주었습니다! |
||
} | ||
|
||
public Object getHandler(final HttpServletRequest request) { | ||
return null; | ||
HandlerKey handlerKey = new HandlerKey(request.getRequestURI(), RequestMethod.valueOf(request.getMethod())); | ||
return handlerExecutions.get(handlerKey); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,11 +2,20 @@ | |
|
||
import jakarta.servlet.http.HttpServletRequest; | ||
import jakarta.servlet.http.HttpServletResponse; | ||
import java.lang.reflect.Method; | ||
import webmvc.org.springframework.web.servlet.ModelAndView; | ||
|
||
public class HandlerExecution { | ||
|
||
private final Method method; | ||
|
||
public HandlerExecution(final Method method) { | ||
this.method = method; | ||
} | ||
|
||
public ModelAndView handle(final HttpServletRequest request, final HttpServletResponse response) throws Exception { | ||
return null; | ||
final Class<?> declaringClass = method.getDeclaringClass(); | ||
final Object object = declaringClass.getDeclaredConstructor().newInstance(); | ||
Comment on lines
+17
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. HandlerExecution 객체가 Object를 필드로 가지게 하는건 어떨까요?! 제가 리플렉션에 대해서 깊게 아는건 아니지만 ㅎㅎ 디노가 작성해주신 것처럼 method에서 클래스 인스턴스를 생성할 수도 있지만, 핸들러를 생성하는 객체에서 함께 인스턴스 생성하는 책임을 가져보는건 어떨지 한번 제안드려봅니다 ㅎㅎ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그레이의 말대로 object를 만드는 책임을 HandlerExcution보다는 외부에서 만들어 가져오는 편이 더 자연스럽다고 생각이 드네요..! 변경했습니다ㅎㅎ |
||
return (ModelAndView) method.invoke(object, request, response); | ||
} | ||
} |
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.
반영하지 않으셔도 됩니다 !!
저는 개인적으로 stream 내부에 stream이 들어가면 가독성이 더 떨어진다고 생각해요 ㅎㅎ
메서드 분리도 하나의 방법이 될 수 있을 것 같은데 디노가 끌리는대로 해주세요 😁
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.
저도 가독성 측면에서 단점이 있다고 생각 해봤는데요.!
함수 분리로 인해서 오히려 로직의 흐름을 한 눈에 파악하기 힘들다는 생각과 함께 고민하다 그대로 두었었습니다.
그래도 한 번 정도는 더 분리를 해도 파악하기엔 어렵지 않다는 생각이 들어서 findRequestMapping이라는 함수를 분리해 보았습니다.!