Skip to content
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단계] 글렌(전석진) 미션 제출합니다. #343

Merged
merged 12 commits into from
Sep 14, 2023

Conversation

seokjin8678
Copy link

@seokjin8678 seokjin8678 commented Sep 13, 2023

안녕하세요 비버!
레벨3때 이후로 간만이네요 😁

Step1 구현하고 미션 제출합니다.

AnnotationHandlerMapping의 흐름은 다음과 같습니다.

  1. 패키지를 인자로 받아, Reflections 객체를 생성
  2. @Controller 어노테이션이 붙은 클래스를 찾음
  3. initializeHandler() 메소드를 통해 Class 타입의 객체를 인스턴스화
  4. @RequestMapping 어노테이션이 붙은 메소드를 찾음
  5. @RequestMapping 속성에 있는 url과 httpMethod로 HandlerKey 객체 생성
  6. 핸들러의 인스턴스와 메소드를 가지고 있는 handlerExecution 객체 생성
  7. HandlerKey 객체를 key, HandlerExecution 객체를 value로 Map에 put

테스트 코드를 추가로 작성하려고 했는데, 기존에 있는 AnnotationHandlerMappingTest에 있는 테스트로 충분한 것 같아 테스트 코드는 작성하지 않았습니다!

리뷰 기다리겠습니다!
감사합니다

리뷰 범위

@seokjin8678 seokjin8678 self-assigned this Sep 13, 2023
Copy link

@ingpyo ingpyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 글랜!

깔끔한 코드 잘봤습니다! 예외부분도 try catch로 잡아 꼼꼼하게 처리해주신 모습이 인상깊네요.
사소한부분 코맨트로 남겨놨습니다!

log.info("Initialized AnnotationHandlerMapping!");
handlerExecutions.keySet()
.forEach(handlerKey -> log.info("Handler Key: {}", handlerKey));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log까지...👍

Comment on lines +63 to +71
Method[] methods = clazz.getDeclaredMethods();
return Arrays.stream(methods)
.filter(method -> method.isAnnotationPresent(RequestMapping.class))
.collect(toSet());
}

private List<HandlerKey> getHandlerKeys(RequestMapping annotation) {
String url = annotation.value();
RequestMethod[] httpMethods = annotation.method();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

재사용하지 않은 부분은 변수추출을 하지않고 바로 사용해보는거는 어떨까요?

@ingpyo ingpyo merged commit 25e0d35 into woowacourse:seokjin8678 Sep 14, 2023
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants