-
Notifications
You must be signed in to change notification settings - Fork 305
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단계] 마코(이규성) 미션 제출합니다. #345
Conversation
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.
안녕하세요 마코
빠르고 깔끔하게 구현해주셔서 코멘트달게 별로 없었어요 (진짜로)
추가 수정사항이나 리팩터링은 2단계라서 그대로 진행해주시면 좋을 것 같습니다
2단계가 훨씬 방대한 걸로 알고 있는데 기대하겠습니다~
Arrays.stream(methods) | ||
.filter(this::supportParameters) | ||
.filter(method -> method.isAnnotationPresent(RequestMapping.class)) | ||
.forEach(method -> putHandlerExecutions(handler, method)); |
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.
https://tecoble.techcourse.co.kr/post/2020-05-14-foreach-vs-forloop/
foreach에 로직이 들어가면 안된다는데 어떻게 생각하시나요
근데 저도 foreach로 map에 넣는 작업을 수행하긴 했습니다
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.
좋은 아티클 공유 감사합니다👍
for-loop보다 짧은 코드로 해결하려고 애써 외면하고 이 정도는 괜찮겠지 하고 타협했던 문제인데 충분히 로직을 빼고 스트림으로 해결할 수 있을 것 같네요. 2단계에서 리팩토링 해보겠습니다~
log.info("Initialized AnnotationHandlerMapping!"); | ||
} | ||
|
||
private boolean supportParameters(final Method method) { |
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.
이거 좀 hip하네요 👍
Hi~ 주드 👋
AnnotationHandlerMappingTest
가 통과하도록 구현했습니다.구현한 방법은 다음과 같습니다.
@Controller
애너테이션이 붙은 클래스들을 찾음.@RequestMapping
이 붙은 메서드들을 찾음.HandlerKey
와HandlerExecution
을 생성하여Map<HandlerKey, HandlerExecution>
에 삽입.잘 부탁드립니다~🙏