[MVC 구현하기 - 1단계] 디노(신종화) 미션 제출합니다. #382
Conversation
|
SonarCloud Quality Gate failed.
|
| final Class<?> declaringClass = method.getDeclaringClass(); | ||
| final Object object = declaringClass.getDeclaredConstructor().newInstance(); |
There was a problem hiding this comment.
HandlerExecution 객체가 Object를 필드로 가지게 하는건 어떨까요?!
제가 리플렉션에 대해서 깊게 아는건 아니지만 ㅎㅎ
Method 객체를 invoke할 때 해당 클래스의 인스턴스가 필요하더라구요 !
디노가 작성해주신 것처럼 method에서 클래스 인스턴스를 생성할 수도 있지만, 핸들러를 생성하는 객체에서 함께 인스턴스 생성하는 책임을 가져보는건 어떨지 한번 제안드려봅니다 ㅎㅎ
There was a problem hiding this comment.
그레이의 말대로 object를 만드는 책임을 HandlerExcution보다는 외부에서 만들어 가져오는 편이 더 자연스럽다고 생각이 드네요..! 변경했습니다ㅎㅎ
| controllerClasses.forEach( | ||
| aClass -> Arrays.stream(aClass.getDeclaredMethods()).forEach( | ||
| method -> { | ||
| final RequestMapping requestMapping = method.getAnnotation(RequestMapping.class); | ||
| if (requestMapping != null) { | ||
| putHandlerExecutions(method, requestMapping); | ||
| } |
There was a problem hiding this comment.
반영하지 않으셔도 됩니다 !!
저는 개인적으로 stream 내부에 stream이 들어가면 가독성이 더 떨어진다고 생각해요 ㅎㅎ
메서드 분리도 하나의 방법이 될 수 있을 것 같은데 디노가 끌리는대로 해주세요 😁
There was a problem hiding this comment.
저도 가독성 측면에서 단점이 있다고 생각 해봤는데요.!
함수 분리로 인해서 오히려 로직의 흐름을 한 눈에 파악하기 힘들다는 생각과 함께 고민하다 그대로 두었었습니다.
그래도 한 번 정도는 더 분리를 해도 파악하기엔 어렵지 않다는 생각이 들어서 findRequestMapping이라는 함수를 분리해 보았습니다.!
| 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.
중요한 부분은 아니지만 stream 최종연산에서 stream 외부에 있는 객체의 접근은 최대한 피하는게 좋다고 합니다 !
(스트림은 함수형 프로그래밍을 위해 등장했기 때문에 비동기 동작시 문제가 될 수 있습니다 🤓)
There was a problem hiding this comment.
어렵네요.. 해당 부분은 다시 for문으로 바꾸어 주었습니다!
안녕하세요 그레이! 디노입니다~🦖
이렇게 하는게 맞는건지 잘 모르겠지만 우선 1단계 요구사항에 대한 구현 완료했습니다..
이상한거나 궁금한게 있으면 뭐든 코멘트 달아주세요!!!
이번 미션 잘 부탁드립니다.! 🙇
step1 변경 사항