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단계] 로지(윤가영) 미션 제출합니다 #354

Merged
merged 2 commits into from
Sep 14, 2023

Conversation

kyY00n
Copy link
Member

@kyY00n kyY00n commented Sep 13, 2023

안녕하세요 호이 :D
mvc 미션 잘 부탁드립니다!!

Copy link

@This2sho This2sho left a comment

Choose a reason for hiding this comment

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

1단계 미션 고생하셨습니다 로지 🙇‍♂️
Stream으로 깔끔하게 잘 구현하셨네요 👍
1단계는 바로 어프로브 하겠습니다 ~!
(미션을 먼저하고 리뷰하고 싶어서 늦었습니다..)

private final Map<HandlerKey, HandlerExecution> handlerExecutions;

public AnnotationHandlerMapping(final Object... basePackage) {
this.basePackage = basePackage;
this.basePackages = basePackage;

Choose a reason for hiding this comment

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

꼼꼼하시네요 👍

final var controllers = Arrays.stream(basePackages)
.flatMap(basePackage -> getControllerFrom(basePackage).stream())
.map(this::newInstance)
.collect(Collectors.toList());

Choose a reason for hiding this comment

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

stream 깔끔하네요 👍

public HandlerExecution(final Object instance, final Method handler) {
this.instance = instance;
this.handler = handler;
}

Choose a reason for hiding this comment

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

별건 아니지만 Method 타입만 받고 다음과 같은 방법으로 Instance를 만드는 방법도 있을 거 같습니다!
handler.getDeclaringClass().getConstructor().newInstance();

@This2sho This2sho merged commit cd832c7 into woowacourse:kyy00n 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.

2 participants