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 구현하기 - 2단계] 레넌(조형래) 미션 제출합니다. #271

Merged
merged 3 commits into from
Sep 27, 2022

Conversation

brorae
Copy link

@brorae brorae commented Sep 26, 2022

안녕하세요 헌치~
이번에도 잘 부탁드려요~

Copy link

@BETTERFUTURE4 BETTERFUTURE4 left a comment

Choose a reason for hiding this comment

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

레넌! 2단계도 수고하셨습니다~👍
이미 adapter를 구현하셨던 만큼 추가되는 코드가 많지 않았고, 깔끔하게 구현되어 있네요!💯
크게 수정할 부분이 없어보여 바로 approve 하겠습니다!
코멘트 단 내용들은 트레이드 오프 영역인 만큼 3단계 때 고려해주세요😉

for (final Class<?> clazz : classes) {
private void addHandlerExecutions(final Map<Class<?>, Object> controllers) {
for (Class<?> clazz : controllers.keySet()) {
final Method[] methods = clazz.getDeclaredMethods();
checkRequestMappingMethod(methods);
final Object controller = controllers.get(clazz);
checkRequestMappingMethod(controller, methods);

Choose a reason for hiding this comment

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

(그냥 의견입니다!)
해당 맵에서 각 세트마다 key, value 모두 사용하고 있네요.
keySet() 대신 entrySet()을 사용하는 방법도 있을 것 같아요.
물론 가독성 면에서 지금처럼 변수로 추출하는 것도 방법입니다. 트레이드 오프일 것 같아요!

@BETTERFUTURE4 BETTERFUTURE4 merged commit 3158356 into woowacourse:brorae Sep 27, 2022
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.

2 participants