[MVC 구현하기 - 1단계] 쥬니(전정준) 미션 제출합니다. #421
Conversation
There was a problem hiding this comment.
안녕하세요 쥬니, 리뷰를 맡은 땡칠입니다 🙇🏻
학습 테스트 모두 잘 진행해주셨고, 코드도 너무 깔끔하게 잘 작성해주셨습니다.
특히 스트림을 활용해서 행위를 집약적으로 잘 나열해주신게 인상깊었습니다.
평소에 어떤 코드를 작성하시는지 잘 보였어요 👀
구현 과정에서, @RequestMapping 어노테이션이 붙어있다고 해서, 모두 HandlerExecution으로 등록할 경우, 예기치 못한 예외가 발생할 것이라고 생각했습니다.
이때, 어디까지 검증을 수행해야하나.. 고민이 많았는데요.
반환 타입 및 파라미터 타입까지는 검증해주자 ! 라는 제 나름의 기준을 세웠습니다.
저는 사실 이 정도까진 생각해보지 못했는데 너무 적합한 방법이라고 생각합니다.
잘못된 메서드가 삽입되어서 예외가 터지기 전에 미리 막을 수 있겠네요! 👍
더 이야기를 나눠보고 싶은데 리뷰할 내용이 많이 없어서 아쉽습니다 ㅠㅠ
미션 고생하셨고 다음 단계에서 뵈어요!
더 궁금한 점은 DM이나 커멘트 남겨주세요!
| private boolean validateParameterType(Class<?>[] parameters) { | ||
| return parameters[REQUEST_INDEX].equals(HttpServletRequest.class) && | ||
| parameters[RESPONSE_INDEX].equals(HttpServletResponse.class); | ||
| } |
There was a problem hiding this comment.
인자 검증까지 해주셨네요 👍
그런데 메서드명을 보면 검증을 통해 예외가 발생할 것 같다는 생각이 들어요.
'boolean 값 반환'이라는 행위에 맞게 메서드명을 수정해보면 어떨까요?
There was a problem hiding this comment.
validateParameterType -> isValidateParameterType 으로 변경했습니다 !
| private List<Method> extractMethods(Class<?> controllerClasses) { | ||
| return Arrays.stream(controllerClasses.getDeclaredMethods()) | ||
| .filter(method -> method.isAnnotationPresent(RequestMapping.class)) | ||
| .filter(method -> method.getReturnType().equals(ModelAndView.class)) | ||
| .filter(method -> validateParameterType(method.getParameterTypes())) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
스트림을 사용해서 깔끔하게 작성해주셨네요.
덕분에 '무엇을 하고 싶은지?'가 정확하게 보여요!
| try { | ||
| for (RequestMethod requestMethod : requestMethods) { | ||
| HandlerKey handlerKey = new HandlerKey(url, requestMethod); | ||
| Object classInstance = extractClassInstance(method); | ||
| HandlerExecution handlerExecution = new HandlerExecution(method, classInstance); | ||
|
|
||
| handlerExecutions.put(handlerKey, handlerExecution); | ||
| } | ||
| } catch (Exception e) { | ||
| log.error("ERROR : {}", e.getMessage()); | ||
| } |
There was a problem hiding this comment.
예외를 처리하는 것도 하나의 행위라고 생각합니다.
그래서 이 부분을 별도의 메서드로 추상화해보면 더 서술적이고 인덴트도 줄어들어서 좋을 것 같아요.
throws Exception이 명시된 함수형 인터페이스 Callable<>를 활용해보시면 어떨까ㅛ?
There was a problem hiding this comment.
extractClassInstance 메서드에서 예외 처리 로직을 수행하도록 수정하였습니다 !
안녕하세요 땡칠 ! 반갑습니다 😄
AnnotationHandlerMappingTest통과에 목적을 두고, 1단계를 구현했습니다.구현 과정에서,
@RequestMapping어노테이션이 붙어있다고 해서, 모두 HandlerExecution으로 등록할 경우, 예기치 못한 예외가 발생할 것이라고 생각했습니다.이때, 어디까지 검증을 수행해야하나.. 고민이 많았는데요.
반환 타입 및 파라미터 타입까지는 검증해주자 ! 라는 제 나름의 기준을 세웠습니다.
리뷰 미리 감사합니다 😉