Skip to content

Step2 구현하기 #36

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

Merged
merged 15 commits into from
Jan 25, 2019
Merged

Step2 구현하기 #36

merged 15 commits into from
Jan 25, 2019

Conversation

leejh903
Copy link

@leejh903 leejh903 commented Jan 24, 2019

step2에서 제시하는 방법대로 리팩토링을 하고 싶었는데 구조가 다르다보니 똑같이 하진 못했습니다.
최대한 이전 코드에서 리팩토링 해보았습니다.
피드백 많이 부탁드립니다!!

Copy link
Contributor

@javajigi javajigi left a comment

Choose a reason for hiding this comment

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

구현 잘 했네요. 💯
Spring의 HandlerMethodArgumentResolver 인터페이스를 참고해 구현해보면서 인터페이스의 맛을 한번 느껴보면 좋겠네요.

@@ -15,40 +15,40 @@
public class ParameterBinder {
Copy link
Contributor

Choose a reason for hiding this comment

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

Spring의 HandlerMethodArgumentResolver 인터페이스를 참고해 구현해 본다.

Copy link
Contributor

@javajigi javajigi left a comment

Choose a reason for hiding this comment

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

MethodParameter가 필요한가? MethodParameter를 추가하지 않고 구현에 도전해 본다.

import java.util.stream.Collectors;

import static org.slf4j.LoggerFactory.getLogger;

public class ParameterBinder {
private static final Logger log = getLogger(ParameterBinder.class);
private static final Map<MethodParameter, HandlerMethodArgumentResolver> parameterBinderHandler = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

MethodParameter없이 List<HandlerMethodArgumentResolver>로 구현할 수 없을까?

@javajigi javajigi merged commit 0377798 into code-squad:brad903 Jan 25, 2019
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