Implemented injector #1012
Implemented injector #1012
Conversation
There was a problem hiding this comment.
Great job on implementing the annotations and the service classes! 🌟 Your use of the @Component and @Inject annotations is on point, and the implementations follow the provided checklist. It's fantastic to see proper exception handling in the FileReaderServiceImpl and adherence to the requirements in the ProductParserImpl and ProductServiceImpl. Keep up the good work! Remember, there's always room for improvement, so consider the feedback for future enhancements. Keep coding and learning! 💪🎉
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| if (clazz == null) { | ||
| throw new RuntimeException(); |
There was a problem hiding this comment.
When throwing a RuntimeException, include a message that describes the error. For example, if no implementation is found for an interface, specify which interface is missing an implementation.
| private Object createNewInstance(Class<?> clazz) { | ||
| if (instances.containsKey(clazz)) { | ||
| return instances.get(clazz); | ||
| } | ||
| try { | ||
| Constructor<?> constructor = clazz.getConstructor(); | ||
| Object instance = constructor.newInstance(); | ||
| instances.put(clazz, instance); | ||
| return instance; | ||
| } catch (ReflectiveOperationException e) { | ||
| throw new RuntimeException(e); | ||
| } |
There was a problem hiding this comment.
Let's check the instances map before new instance creation to avoid unnecessary object creation. This logic is already present but could be improved by moving the 'instances' map check into the 'createNewInstance' method to ensure it's always checked before creating a new instance.
| Map<Class<?>, Class<?>> interfaceImplementations = new HashMap<>(); | ||
| interfaceImplementations.put(FileReaderService.class, FileReaderServiceImpl.class); | ||
| interfaceImplementations.put(ProductParser.class, ProductParserImpl.class); | ||
| interfaceImplementations.put(ProductService.class, ProductServiceImpl.class); |
There was a problem hiding this comment.
Make the 'interfaceImplementations' map a class field and initialize it using 'Map.of()' for better performance and cleaner code.
| field.setAccessible(true); | ||
| field.set(clazzInstance, fieldInstance); | ||
| } catch (IllegalAccessException e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
When catching an exception, it is better to replace many exceptions that have a common parent with a general parental exception. In this case, 'ReflectiveOperationException' could be used instead of 'IllegalAccessException'.
| public Object getInstance(Class<?> interfaceClazz) { | ||
| Object clazzImplInstance = null; | ||
| Class<?> clazz = findImplementation(interfaceClazz); | ||
| if (clazz == null) { | ||
| throw new RuntimeException(); | ||
| } | ||
| Field[] declaredFields = clazz.getDeclaredFields(); | ||
| for (Field field : declaredFields) { | ||
| if (field.isAnnotationPresent(Inject.class)) { | ||
| Object fieldInstance = getInstance(field.getType()); | ||
| Object clazzInstance = createNewInstance(clazz); | ||
| try { | ||
| field.setAccessible(true); | ||
| field.set(clazzInstance, fieldInstance); | ||
| } catch (IllegalAccessException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } | ||
| } | ||
| if (clazzImplInstance == null) { | ||
| clazzImplInstance = createNewInstance(clazz); | ||
| } | ||
| return clazzImplInstance; |
There was a problem hiding this comment.
The 'clazzImplInstance' object is redundant and can be removed. Instead, you can return the instance directly from the 'createNewInstance' method after checking for the '@component' annotation.
| public Object getInstance(Class<?> interfaceClazz) { | ||
| Object clazzImplInstance = null; | ||
| Class<?> clazz = findImplementation(interfaceClazz); | ||
| if (clazz == null) { | ||
| throw new RuntimeException(); | ||
| } | ||
| Field[] declaredFields = clazz.getDeclaredFields(); | ||
| for (Field field : declaredFields) { | ||
| if (field.isAnnotationPresent(Inject.class)) { | ||
| Object fieldInstance = getInstance(field.getType()); | ||
| Object clazzInstance = createNewInstance(clazz); | ||
| try { | ||
| field.setAccessible(true); | ||
| field.set(clazzInstance, fieldInstance); | ||
| } catch (IllegalAccessException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } | ||
| } | ||
| if (clazzImplInstance == null) { | ||
| clazzImplInstance = createNewInstance(clazz); | ||
| } | ||
| return clazzImplInstance; |
There was a problem hiding this comment.
You need to add checks for the '@component' annotation in your Injector class to ensure that you are only creating instances of classes that have this annotation.
| @Inject | ||
| private Long id; | ||
| @Inject | ||
| private String name; | ||
| @Inject | ||
| private String category; | ||
| @Inject | ||
| private String description; | ||
| @Inject | ||
| private BigDecimal price; |
There was a problem hiding this comment.
The @Inject annotation should not be used on model class fields like id, name, category, description, and price. It is typically used to mark a dependency that is to be injected into a class by an injector (DI framework), and model classes usually do not have such dependencies.
No description provided.