Skip to content

Gramway framework2 add DI #31

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 10 commits into from
Aug 1, 2018
Merged

Conversation

durin93
Copy link

@durin93 durin93 commented Jul 30, 2018

DI 에 대해 아직 이해가 부족해 맞는지 잘 모르겠지만 일단 구현해 보았습니다.

@repository 어노테이션을 붙이면 BeanFactory에 해당 인스턴스가 한번만 생성되서 저장이 되고,
@Autowired 어노테이션을 필드에 붙이면 해당 필드 값에 BeanFactory에서 해당 인스턴스를 주입해주는 방식으로 구현하였습니다.

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.

특별한 힌트 없이 구현 잘 했다. 진심. 👍
피드백 몇 개 남겼는데 좀 더 개선해 봐라.

if (clazz.isAnnotationPresent(Controller.class)) {
addController(clazz);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

static으로 구현하기 보다 init()과 같은 메소드를 만들고 호출하는 구조면 어떨까?


if (directory.exists()) {
scan(directory.listFiles(), classes, ROOT);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

이렇게 수동으로 구현하지 말고 reflections 라이브러리 적용해서 구현해 봐라.

public static void addBeans(List<Class<?>> classes) {
for (Class<?> clazz : classes) {
addControllerBean(clazz);
addRepository(clazz);
Copy link
Contributor

Choose a reason for hiding this comment

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

Controller, Repository만이 아니라 추후 Service, Component와 같은 애노테이션도 추가할 수 있도록 확장 가능한 구조로 개선해 보면 어떨까? 애노테이션의 배열을 전달하면 해당 클래스를 모두 찾아 Bean으로 등록..

이후에 inject

@javajigi javajigi merged commit 431fd17 into code-squad:durin93 Aug 1, 2018
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