[DI - Step 2] 효오 미션 제출합니다. #49

hyojaekim · 2019-11-11T13:28:24Z

안녕하세요!
월~목 동미참으로 인해 빠르게 미션 제출하겠습니다!

BeanDefinition 클래스를 만들어서 Method를 Bean을 관리하는 MethodoBeanDefinition, 기본 어노테이션(Controller, Service, Repoository)를 관리하는 DefaultBeanDefinition을 구현했습니다.
어떤지 궁금합니다.

많은 지적 부탁드리겠습니다!

- getTypesAnnotated 메소드 로직 변경

- BeanFactory -> beanDefinition 필드 & register 메소드 추가

- BeanScanner -> ClasspathBeanScanner 이름 변경 - DefaultBeanDefinition 추가

- JdbcTemplate Inject

kingbbode

안녕하세요!
리뷰가 많이 늦었습니다ㅜㅜ
몇 가지 피드백 작성해두었습니다 :)

kingbbode · 2019-11-17T13:59:21Z

nextstep-di/src/main/java/nextstep/di/factory/BeanFactory.java

+        Optional<BeanDefinition> maybeBeanDefinition = this.beanDefinitions.stream()
+                .filter(beanDefinition -> beanDefinition.getBeanClass().equals(concreteClass))
+                .findAny();
+        if (maybeBeanDefinition.isPresent()) {
+            return maybeBeanDefinition.get();
+        }

-        if (!beans.containsKey(preInstanticateBean)) {
-            beans.put(preInstanticateBean, createBean(preInstanticateBean));
+        if (concreteClass.isInterface()) {
+            return findBeanDefinition(findConcreteClass(concreteClass));
        }
+        throw new RuntimeException();


concreteClass.isInterface() 를 먼저 검사해보는 것은 어떨까요? 예외되는 상황을 먼저 처리하기도 하고, 먼저 처리되기에도 충분하여 가독이 더 좋아질 것 같아요 :)

피드백 내용을 토대로 안티 패턴을 제거했더니, 훨씬 가독성이 좋아졌습니다!

kingbbode · 2019-11-17T14:02:28Z

nextstep-di/src/main/java/nextstep/di/factory/BeanFactory.java

        }
+        Set<Class<?>> beanClazz = this.beanDefinitions.stream()


매번 수행하는 것은 비효율적이지 않을까요?

kingbbode · 2019-11-17T14:14:21Z

nextstep-di/src/main/java/nextstep/di/factory/BeanFactory.java

+    private void createDefaultBean(DefaultBeanDefinition beanDefinition) {
+        Class<?> beanClass = beanDefinition.getBeanClass();
+
+        beans.put(beanClass, createBean(beanClass));
+    }
+
+    private void createMethodBean(MethodBeanDefinition beanDefinition) {
+        Class<?> beanClass = beanDefinition.getBeanClass();
+        Object implementation = beanDefinition.getImplementation();
+        Method method = beanDefinition.getMethod();
+
+        Object[] parameterBeans = Arrays.stream(method.getParameterTypes())
+                .map(this::getBean)
+                .toArray();
+        try {
+            beans.put(beanClass, method.invoke(implementation, parameterBeans));
+        } catch (IllegalAccessException | InvocationTargetException e) {
+            throw new RuntimeException(e);
        }
    }


DefaultBeanDefinition, MethodBeanDefinition 의 생성 공통점으로 추상화하여 BeanFactory 가 구현을 모르도록 설계해볼 수 있을 것 같아요 :)

BeanDefinition에서 빈을 생성하도록 수정하였더니 BeanFactory의 복잡도가 줄어드는 장점이 있었습니다!

좋은 피드백 감사합니다!

kingbbode · 2019-11-17T14:17:31Z

nextstep-di/src/test/java/nextstep/di/factory/BeanFactoryTest.java

+        ClasspathBeanScanner classpathBeanScanner = new ClasspathBeanScanner(beanFactory);
+        classpathBeanScanner.doScan("nextstep.di.factory.example");


BeanFactory 를 테스트하는데 ClasspathBeanScanner가 필요할까요?

기존에는 Bean을 등록하려면 Scanner에 BeanFactory를 받아 Bean들을 등록하도록 로직을 수행했습니다.

리팩토링 후에는 Scanner들을 추상화하여 BeanFactory 생성자에서 Scanner를 이용해서 BeanClass들을 찾고, 빈을 등록하도록 수정했는데 괜찮은지 궁금합니다.

kingbbode · 2019-11-17T14:18:38Z

nextstep-di/src/test/java/nextstep/di/factory/ConfigurationBeanScannerTest.java

+        cbs.register(ExampleConfig.class);
+        beanFactory.initialize();
+
+        assertNotNull(beanFactory.getBean(DataSource.class));


Scanner 를 테스트하는데 BeanFactory의 initialize 까지 필요한가요?

- initialize -> 생성자에서 수행하도록 변경 - ApplicationContext 필드 제거

- BeanFactory -> BeanDefinition에서 빈 생성하도록 수정

- Scanner에서 BeanFactory에 빈 등록 -> BeanFactory에서 Scanner를 이용하여 빈 등록

kingbbode

피드백 잘 반영해주셨네요! 👍
추가로 몇가지 피드백 남겼습니다 :)
확인 부탁드려요!

kingbbode · 2019-11-26T13:17:31Z

nextstep-di/src/main/java/nextstep/di/factory/BeanFactory.java

+        if (concreteClass.isInterface() && notExistBeanDefinition(concreteClass)) {
            return findBeanDefinition(findConcreteClass(concreteClass));
        }
-        throw new RuntimeException();
+
+        return beanDefinitions.stream()
+                .filter(beanDefinition -> beanDefinition.sameBeanClass(concreteClass))
+                .findAny()
+                .orElseThrow(RuntimeException::new);
+    }
+
+    private boolean notExistBeanDefinition(Class<?> concreteClass) {
+        return beanDefinitions.stream()
+                .noneMatch(beanDefinition -> beanDefinition.sameBeanClass(concreteClass));
    }


잘 분리하셨네요! :)

그런데 beanDefinition 을 매번 순회하는 중복이 발생하였네요.
beanDefinitions 를 Set이 아닌 다른 자료형으로 저장해놓으면 더 효율적이지 않을까요?

kingbbode · 2019-11-27T01:46:42Z

nextstep-di/src/main/java/nextstep/di/factory/BeanDefinition.java

@@ -0,0 +1,21 @@
+package nextstep.di.factory;
+
+public abstract class BeanDefinition {


interface 로도 충분하지 않을까요?

https://hamait.tistory.com/650

감사합니다! 혹시 킹뽀대님은 추상클래스와 인터페이스를 어떤 기준으로 사용하고 계시는지 궁금합니다!

첨부한 링크의 마지막 부분의 내용과 동일한 기준으로 사용하고 있습니다 :)

kingbbode · 2019-11-27T02:10:15Z

nextstep-di/src/main/java/nextstep/di/factory/BeanFactory.java

-import java.lang.reflect.InvocationTargetException;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.Arrays;
 import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;

 public class BeanFactory {


BeanFactoryUtils 내부에서 interface 여부를 검증하고 ConcreteClass를 찾아주고 있습니다.
이미 해주고 있는 일에 대한 검증과 행위에 대한 중복을 제거해보면 좋을 것 같아요 :)

- 중복 및 불필요한 로직 제거 - BeanFactoryUtils 메소드 파라미터 타입 변경 및 체크로직 추가

woowahankingbbode

외부 출장 일정으로 리뷰가 많이 늦었네요 ㅜㅜ
수고 많으셨습니다! 👍
몇가지 코멘트 작성해두었으니 확인 부탁해요 :)

woowahankingbbode · 2019-12-11T04:51:15Z

nextstep-di/src/main/java/nextstep/di/factory/DefaultBeanDefinition.java

+
+import java.lang.reflect.Constructor;
+
+public class DefaultBeanDefinition implements BeanDefinition {


다른 구현체가 MethodBeanDefinition 이니 이 구현체는 ConstructorBeanDefinition 정도의 네이밍이 적절하지 않을까요?

woowahankingbbode · 2019-12-11T04:52:07Z

nextstep-di/src/main/java/nextstep/di/factory/BeanFactory.java

-        BeanScanner beanScanner = new BeanScanner(basePackage);
-        this.preInstanticateBeans = beanScanner.getTypesAnnotated();
-    }
+    private List<BeanDefinition> beanDefinitions = Lists.newArrayList();


Map 타입의 BeanDefinition 으로 매번 순회하지 않는 것을 의도하였는데 너무 돌려서 이야기했던 것 같네요 :)

hyojaekim added 9 commits November 11, 2019 21:59

refactor: 불필요한 메소드 삭제 및 로그 추가

5cd10f9

- getTypesAnnotated 메소드 로직 변경

feat: ConfigurationBeanScanner, MethodBeanDefinition 추가

03fa26b

- BeanFactory -> beanDefinition 필드 & register 메소드 추가

feat: BeanDefinition 추상 클래스 생성

2ab1c02

feat: ConfigurationBeanScanner 빈 등록 구현

4f8d363

refactor: ClasspathBeanScanner

907e65b

- BeanScanner -> ClasspathBeanScanner 이름 변경 - DefaultBeanDefinition 추가

feat: ApplicationContext 생성, ComponentScan 구현

d42caf3

feat: ApplicationContext, ComponentScan 적용

7fcfc07

feat: MyConfiguration 추가 및 적용

3a1a82d

- JdbcTemplate Inject

fix: 어노테이션 적용에 따른 테스트 코드 수정

11cee17

kingbbode suggested changes Nov 17, 2019

View reviewed changes

hyojaekim added 4 commits November 23, 2019 16:10

refactor: ApplicationContext initialize 로직 이동

705d3b6

- initialize -> 생성자에서 수행하도록 변경 - ApplicationContext 필드 제거

refactor: BeanFactory 안티패턴 제거

8d37879

refactor: BeanDefinition 빈 생성

94cabbb

- BeanFactory -> BeanDefinition에서 빈 생성하도록 수정

refactor: BeanScanner 인터페이스 적용

c9307e8

- Scanner에서 BeanFactory에 빈 등록 -> BeanFactory에서 Scanner를 이용하여 빈 등록

kingbbode suggested changes Nov 27, 2019

View reviewed changes

hyojaekim added 3 commits November 27, 2019 20:47

refactor: 메서드 리턴 타입 및 자료형 변경

67fbb2b

refactor: BeanFactory beanClasses 필드 추가

2ed576c

- 중복 및 불필요한 로직 제거 - BeanFactoryUtils 메소드 파라미터 타입 변경 및 체크로직 추가

refactor: BeanDefinition interface 적용

a02eebd

woowahankingbbode approved these changes Dec 11, 2019

View reviewed changes

kingbbode merged commit 90a7a53 into woowacourse:hyojaekim Dec 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DI - Step 2] 효오 미션 제출합니다. #49

[DI - Step 2] 효오 미션 제출합니다. #49

hyojaekim commented Nov 11, 2019

kingbbode left a comment

kingbbode Nov 17, 2019

hyojaekim Nov 25, 2019

kingbbode Nov 17, 2019

kingbbode Nov 17, 2019 •

edited

Loading

hyojaekim Nov 25, 2019

kingbbode Nov 17, 2019

hyojaekim Nov 26, 2019 •

edited

Loading

kingbbode Nov 17, 2019

kingbbode left a comment

kingbbode Nov 26, 2019

kingbbode Nov 27, 2019

hyojaekim Nov 30, 2019

kingbbode Dec 11, 2019

kingbbode Nov 27, 2019

woowahankingbbode left a comment

woowahankingbbode Dec 11, 2019

woowahankingbbode Dec 11, 2019

		ClasspathBeanScanner classpathBeanScanner = new ClasspathBeanScanner(beanFactory);
		classpathBeanScanner.doScan("nextstep.di.factory.example");

		@@ -0,0 +1,21 @@
		package nextstep.di.factory;

		public abstract class BeanDefinition {


		import java.lang.reflect.Constructor;

		public class DefaultBeanDefinition implements BeanDefinition {

[DI - Step 2] 효오 미션 제출합니다. #49

[DI - Step 2] 효오 미션 제출합니다. #49

Conversation

hyojaekim commented Nov 11, 2019

kingbbode left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kingbbode Nov 17, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hyojaekim Nov 26, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kingbbode left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

woowahankingbbode left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kingbbode Nov 17, 2019 •

edited

Loading

hyojaekim Nov 26, 2019 •

edited

Loading