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

[DI - Step 1] 베디 미션 제출합니다. #17

Merged
merged 10 commits into from
Nov 2, 2019
Merged

Conversation

dpudpu
Copy link

@dpudpu dpudpu commented Nov 1, 2019

브리 안녕하세요.

어려운 주제도 부담없이 주시면 감사하겠습니다. 리뷰 잘 부탁드립니다!
감사합니다.

Copy link

@byjo byjo left a comment

Choose a reason for hiding this comment

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

베디 안녕하세요~ 이번 미션 리뷰를 맡은 브리입니다.
크게 피드백 드릴 내용이 없을 정도로 요구사항 구현 및 테스트, 적용까지 잘 진행 하셨어요. 👏
이번에 만든 빈 생성 방법을 어떻게 추상화 할 수 있을지를 고민하면서 2단계 미션 진행하시면 좋을 것 같아요.
다음 단계 코드도 기대해 보겠습니다!

preInstantiatedBeans.forEach(this::createBean);
}

private Object createBean(final Class<?> preInstantiatedBean) {
Copy link

Choose a reason for hiding this comment

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

메소드 분리가 잘 되어 있어 빈 생성 로직이 한 눈에 잘 들어오네요 💯💯

Optional<Constructor<?>> injectedConstructor = BeanFactoryUtils.getInjectedConstructor(concreteClass);
return injectedConstructor
.orElseGet(() -> {
try {
Copy link

Choose a reason for hiding this comment

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

stream API 사용 시 checked exception이 있으면,
람다를 사용했을 때 기대하는 가독성이란 장점을 100% 살리기 어려운 것 같아요.
이 때 도움이 되었던 아티클 하나 공유드립니다!

@byjo byjo merged commit 77a7ab6 into woowacourse:dpudpu Nov 2, 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.

3 participants