-
Notifications
You must be signed in to change notification settings - Fork 998
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
Jv dependency injection solution #699
base: main
Are you sure you want to change the base?
Jv dependency injection solution #699
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good solution! Let's try to improve it
break; | ||
} | ||
} | ||
System.out.println(clazzInterface + " " + interfaceImplementation); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why you are printing results?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
used for quick check, forgot to delete)
public static List<Class<?>> findClassesByPackage(String packageName) { | ||
String path = packageName.replace('.', '/'); | ||
ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); | ||
File packageDirectory = new File(classLoader.getResource(path).getFile()); | ||
File[] files = packageDirectory.listFiles(); | ||
return Arrays.stream(files) | ||
.filter(File::isFile) | ||
.filter(file -> file.getName().endsWith(CLASS_INDICATOR)) | ||
.map(file -> getClass(file.getName(), packageName)) | ||
.collect(Collectors.toList()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public methods should be before private
if (interfaceClazz.isInterface()) { | ||
if (!interfacesImplementations.containsKey(interfaceClazz)) { | ||
throw new RuntimeException("Can`t find interface " + interfaceClazz.getName()); | ||
} | ||
Class<?> implementation = interfacesImplementations.get(interfaceClazz); | ||
if (implementation == null) { | ||
throw new RuntimeException("Interface " + interfaceClazz.getName() | ||
+ " has no implementations or they doesn't annotated by @Component"); | ||
} | ||
return implementation; | ||
} | ||
componentCheck(interfaceClazz); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should decide how to check exceptions: in separate methods or in this method.
public class Injector { | ||
private static final Injector injector = new Injector(); | ||
private static final Map<Class<?>, Class<?>> interfacesImplementations; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my point of view, we need to follow KISS principles and use just "Map.of(...)" for interface implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greate solution!
But need some improvments
private static Map<Class<?>, Class<?>> interfacesImplementationsConfig() { | ||
Map<Class<?>, Class<?>> map = new HashMap<>(); | ||
map.put(FileReaderService.class, FileReaderServiceImpl.class); | ||
map.put(ProductParser.class, ProductParserImpl.class); | ||
map.put(ProductService.class, ProductServiceImpl.class); | ||
return map; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Common mistakes: "Make Interface Implementations map a class field. You can fill it in using Map.of()"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor fix required
} | ||
} | ||
|
||
private static Class<?> findImplementation(Class<?> interfaceClazz) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why all private methods is static?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 👍
No description provided.