Solution - #1254
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still critical, blocking issues with the DI core classes and package structure. In src/main/java/mate/academy/lib, the four key files (Component, Inject, InjectionException, Injector) are effectively empty, while the rest of the project imports mate.academy.lib.*; this means the actual package being used does not contain the required annotations, custom exception, or injector implementation and will not meet the task requirements or may fail at compile/run time. This also violates the requirement to have a single, consistent set of @Component and @Inject annotations [CHECKLIST ITEM #2] and to implement them with @Retention(RUNTIME) and proper @Target [CHECKLIST ITEM #3].
The critical fixes you need to make are:
- Either remove the duplicate
mate/academy/libpackage entirely, or copy your working implementations into this package with the correctpackage mate.academy.lib;declaration so thatComponent,Inject,InjectionException, andInjectorare fully implemented in the package thatMainand services import. - In your
Injectorconstructor, replace the genericcatch (Exception e) { throw new RuntimeException(...) }with throwing your customInjectionExceptionand include an informative message plus the original cause [CHECKLIST ITEM #7].
Minor improvements (not blockers): you may want to separate the error messages for “no implementation found” vs “implementation missing @Component” to make debugging clearer, and consider using Map.of(...) if you decide to hard-code interface–implementation mappings. Your overall DI logic, annotations, and exception handling in getInstance look good, so once you fix the duplicate package and constructor exception handling, your solution should fully satisfy the assignment.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| @@ -8,9 +8,8 @@ | |||
| public class Main { | |||
|
|
|||
| public static void main(String[] args) { | |||
There was a problem hiding this comment.
The custom @Component annotation needs to live in the same package that the rest of the codebase imports (mate.academy.lib); make sure you are not accidentally maintaining duplicate Component definitions in different packages, as this can cause runtime confusion and violate the requirement to use only one @Component annotation (checklist item #2).
| @@ -8,9 +8,8 @@ | |||
| public class Main { | |||
|
|
|||
| public static void main(String[] args) { | |||
There was a problem hiding this comment.
Similarly to @Component, ensure this @Inject annotation in mate.academy.lib is the single one used across the project and that no parallel, unused Inject annotation exists elsewhere, otherwise it can conflict with the requirement to use only @Component and @Inject (checklist item #2).
| } catch (Exception e) { | ||
| throw new RuntimeException("Can't initialize injector", e); |
There was a problem hiding this comment.
Catching a broad Exception and throwing a plain RuntimeException here does not use your custom InjectionException and slightly conflicts with checklist item #7, which asks for informative messages and propagating the caught exception in the custom exception type; consider replacing this with throw new InjectionException("Can't initialize injector", e);.
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
This file is completely empty; per the task description, Component must be a custom annotation with @Retention(RUNTIME) and @Target, so this file is missing the required implementation.
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
This file is completely empty; per the task description, Inject must be a custom annotation with @Retention(RUNTIME) and @Target, so this file is missing the required implementation.
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
The core Injector class is missing in this package, but Main and services expect mate.academy.lib.Injector. You must implement the injector here with the required behavior: using @Component and @Inject, checking @Component presence, maintaining instances and implementation maps, and handling reflection errors as described in the task and checklist.
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
This file is empty but is supposed to define the @Component annotation for the DI framework. This violates checklist item #3: "Do not forget to add @Target to custom annotations" and the description requiring a @Component annotation with proper @Retention and @Target. You need to implement this annotation here or remove this duplicate package and rely on the properly implemented one.
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
This file is empty but should define the @Inject annotation. As with Component, this violates checklist item #3 (missing @Target) and the requirement to configure annotations with @Retention so they can be used via reflection. Implement @Inject here (with @Target(ElementType.FIELD) and @Retention(RetentionPolicy.RUNTIME)) or remove this unused duplicate package.
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
InjectionException is not defined here, so any code importing mate.academy.lib.InjectionException from this package will fail. The task requires a custom exception with informative messages and the ability to wrap causes (checklist item #7). You should either implement InjectionException here with the required constructors or ensure you do not have this empty duplicate file/package.
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
The Injector class is completely missing in this package. The core assignment requires an injector that uses @Component and @Inject, manages instances and interface-implementation mappings, checks annotations, and throws InjectionException with informative messages and consolidated reflection exceptions (checklist items #4, #5, #6, #7, #8). You need to either move your working Injector implementation into this package or remove this empty duplicate file/package. As-is, the project does not meet the core functional requirements in this package.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is still a critical structural issue with your DI core package that violates the task requirements and can break compilation. You currently have a duplicate, invalid source directory src/main/java/mate.academy/lib where InjectionException.java (and likely Component.java, Inject.java, Injector.java) are empty, while the real, fully implemented versions live under src/main/java/mate/academy/lib. This conflicts with the requirement to have a single, consistent set of DI classes and annotations in the mate.academy.lib package actually imported by the project [CHECKLIST ITEM #2], and also means the package in src/main/java/mate.academy/lib does not contain a working InjectionException as required [CHECKLIST ITEM #7].
Critical issues to fix:
- Remove the
src/main/java/mate.academy/...directory entirely from the source set, or copy your workingComponent,Inject,InjectionException, andInjectorimplementations fromsrc/main/java/mate/academy/libinto that exact package so they all sharepackage mate.academy.lib;and have full bodies. - Ensure
src/main/java/mate.academy/lib/InjectionException.java(if it remains) contains the same implementation as your workingInjectionException(constructors with informative message and message+cause) instead of being empty.
Once you consolidate to a single, properly implemented mate.academy.lib package, your existing Main, FileReaderServiceImpl, ProductParserImpl, and ProductServiceImpl should work correctly with the injector and annotations, because their DI usage and exception handling are already well done. Keep up the good work—after this package cleanup, your solution should be ready to pass.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib; | |||
There was a problem hiding this comment.
InjectionException is missing its class definition here. This violates the requirement to have a fully implemented custom exception in the package actually used by the project, and also breaks checklist item #7 about throwing informative custom exceptions with causes attached.
| @@ -1,9 +1,11 @@ | |||
| package mate.academy.service.impl; | |||
There was a problem hiding this comment.
InjectionException is declared in the mate.academy.lib package but the class body is empty. This violates the description and checklist item #7, which require a custom exception implementation with informative messages and an overload that accepts a cause; you need to implement this class (or copy the working version from your other mate/academy/lib folder) in this package.
| import mate.academy.lib.Component; | ||
| import mate.academy.model.Product; | ||
| import mate.academy.service.ProductParser; | ||
|
|
There was a problem hiding this comment.
This class relies on mate.academy.lib.Component, but in your project the src/main/java/mate.academy/lib package is not fully implemented while a different mate/academy/lib folder contains the actual annotation. Having duplicate/empty packages breaks the requirement for a single, consistent set of annotations (checklist item #2) and means this import may not resolve correctly; you should consolidate to one mate/academy/lib package with the implemented Component annotation.
| package mate.academy.service.impl; | ||
|
|
||
| import java.math.BigDecimal; | ||
| import mate.academy.lib.Component; |
There was a problem hiding this comment.
This class also imports mate.academy.lib.Component from the partially implemented mate.academy/lib folder. As with FileReaderServiceImpl, you need to ensure that the Component annotation implementation with proper @Retention(RUNTIME) and @Target is present in this exact package, not only in a different folder (checklist items #2 and #3).
| import mate.academy.lib.Component; | ||
| import mate.academy.lib.Inject; | ||
| import mate.academy.model.Product; | ||
| import mate.academy.service.FileReaderService; | ||
| import mate.academy.service.ProductParser; | ||
| import mate.academy.service.ProductService; | ||
|
|
||
| @Component |
There was a problem hiding this comment.
ProductServiceImpl correctly uses @Component on the implementation and @Inject on its dependencies. Just be aware that these annotations must come from the fully implemented mate.academy.lib package (which you have under mate/academy/lib), and you should remove the duplicate, partially implemented mate.academy/lib directory to avoid package conflicts (checklist item #2 about using only a single set of annotations).
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
| import mate.academy.lib.Component; | ||
| import mate.academy.lib.Inject; | ||
| import mate.academy.model.Product; | ||
| import mate.academy.service.FileReaderService; |
There was a problem hiding this comment.
InjectionException is properly implemented with both a message-only and a message+cause constructor, which addresses checklist item #7 about throwing exceptions with informative messages and attaching the caught exception as the cause.
| package mate.academy.lib; | ||
|
|
||
| public class InjectionException extends RuntimeException { | ||
| public InjectionException(String message) { | ||
| super(message); | ||
| } | ||
|
|
||
| public InjectionException(String message, Throwable cause) { | ||
| super(message, cause); | ||
| } |
There was a problem hiding this comment.
| package mate.academy.lib; | ||
|
|
||
| public class InjectionException extends RuntimeException { | ||
| public InjectionException(String message) { | ||
| super(message); | ||
| } | ||
|
|
||
| public InjectionException(String message, Throwable cause) { | ||
| super(message, cause); | ||
| } |
| public class InjectionException extends RuntimeException { | ||
| public InjectionException(String message) { | ||
| super(message); | ||
| } | ||
|
|
||
| public InjectionException(String message, Throwable cause) { |
There was a problem hiding this comment.
InjectionException is correctly implemented with constructors that accept an informative message and a message plus cause, satisfying checklist item #7 about propagating the original exception as the cause; ensure this is the only InjectionException class on the classpath by removing the empty duplicate in src/main/java/mate.academy/lib.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational override so you can continue progressing while you learn, even though there is still one important structural issue to fix.
You did a very good job with the DI core under mate.academy.lib: @Component and @Inject have @Retention(RUNTIME) and proper @Target [CHECKLIST ITEM #3], the Injector keeps interfaceImplementations and instances as class fields and checks the cache before creating new objects [CHECKLIST ITEM #5, #6], and InjectionException is used with clear, informative messages and causes [CHECKLIST ITEM #7]. The services and Main class use the injector correctly, and your naming is clear and readable.
The main issue to improve is structural: under src/main/java/mate/academy/lib you still have four placeholder files (Component.java, Inject.java, InjectionException.java, Injector.java) that only declare package mate.academy.lib.ignore; and have no implementations. This violates the requirement to have a single, consistent mate.academy.lib package with working DI classes, and can cause package and build confusion. To fix this, either remove the whole src/main/java/mate/academy/lib directory from the source set, or replace those stubs with your real implementations (ensuring the package is package mate.academy.lib;).
Overall, your DI logic and usage are strong; once you clean up that duplicate package, your solution will be robust and fully aligned with the requirements.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| @Retention(RetentionPolicy.RUNTIME) | ||
| @Target(ElementType.TYPE) | ||
| public @interface Component { |
There was a problem hiding this comment.
The annotations are correctly configured with @Retention(RetentionPolicy.RUNTIME) and @Target(ElementType.TYPE), which satisfies the requirement to make them usable via reflection at runtime.
| @Retention(RetentionPolicy.RUNTIME) | ||
| @Target(ElementType.TYPE) | ||
| public @interface Component { |
There was a problem hiding this comment.
The annotations are correctly configured with @Retention(RetentionPolicy.RUNTIME) and @Target(ElementType.FIELD), which satisfies the requirement to make them usable via reflection at runtime.
| @Retention(RetentionPolicy.RUNTIME) | ||
| @Target(ElementType.FIELD) |
There was a problem hiding this comment.
The annotation is correctly marked with @Retention(RetentionPolicy.RUNTIME) and @Target(ElementType.TYPE), so it meets the requirements for runtime reflection and checklist item "Do not forget to add @Target to custom annotations." No changes needed here.
| private final Map<Class<?>, Object> instances = new HashMap<>(); | ||
| private final Map<Class<?>, Class<?>> interfaceImplementations = new HashMap<>(); | ||
|
|
||
| private Injector() { | ||
| try { | ||
| List<Class<?>> classes = getClasses("mate.academy"); | ||
| for (Class<?> clazz : classes) { | ||
| if (clazz.isAnnotationPresent(Component.class)) { | ||
| Class<?>[] interfaces = clazz.getInterfaces(); | ||
| if (interfaces.length > 0) { | ||
| interfaceImplementations.put(interfaces[0], clazz); | ||
| } | ||
| } | ||
| } | ||
| } catch (Exception e) { | ||
| throw new InjectionException("Can't initialize injector", e); | ||
| } | ||
| } | ||
|
|
||
| public static Injector getInjector() { | ||
| return injector; | ||
| } | ||
|
|
||
| public Object getInstance(Class<?> interfaceClazz) { | ||
| return null; | ||
| if (instances.containsKey(interfaceClazz)) { | ||
| return instances.get(interfaceClazz); |
There was a problem hiding this comment.
Using the instances map as a class field and checking it before creating new instances satisfies checklist item: "Let's check instances map before new instance creation." No change needed here.
| private final Map<Class<?>, Class<?>> interfaceImplementations = new HashMap<>(); | ||
|
|
||
| private Injector() { | ||
| try { | ||
| List<Class<?>> classes = getClasses("mate.academy"); | ||
| for (Class<?> clazz : classes) { | ||
| if (clazz.isAnnotationPresent(Component.class)) { | ||
| Class<?>[] interfaces = clazz.getInterfaces(); | ||
| if (interfaces.length > 0) { | ||
| interfaceImplementations.put(interfaces[0], clazz); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Populating interfaceImplementations in the constructor and keeping it as a class field satisfies the requirement: "Make Interface Implementations map a class field. You can fill it in using Map.of()." Although you use new HashMap<>() instead of Map.of(), that’s acceptable as long as it stays a field and is initialized centrally.
| } | ||
| instances.put(interfaceClazz, instance); | ||
| return instance; | ||
| } catch (ReflectiveOperationException e) { |
There was a problem hiding this comment.
Catching ReflectiveOperationException instead of multiple specific reflection exceptions matches the checklist advice: "It is better to replace many exceptions that have a common parent with a general parental exception."
| } catch (Exception e) { | ||
| throw new InjectionException("Can't initialize injector", e); |
There was a problem hiding this comment.
Throwing InjectionException with a detailed message and passing e as the cause matches checklist item #7: you include an informative message and rethrow with the original exception as cause.
| import java.lang.reflect.Constructor; | ||
| import java.lang.reflect.Field; | ||
| import java.net.URL; | ||
| import java.util.ArrayList; | ||
| import java.util.Enumeration; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| public class Injector { | ||
| private static final Injector injector = new Injector(); | ||
| private final Map<Class<?>, Object> instances = new HashMap<>(); | ||
| private final Map<Class<?>, Class<?>> interfaceImplementations = new HashMap<>(); |
There was a problem hiding this comment.
The annotation usage here (@Component and @Inject) follows the requirement to use only these custom annotations and is correctly applied to a service implementation and its injectable fields.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
This file only declares package mate.academy.lib.ignore; and does not define the @Component annotation in the mate.academy.lib package. The requirements demand a working Component annotation in package mate.academy.lib;, so this stub should either be removed or replaced with the real implementation.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
Similarly, this stub file defines only package mate.academy.lib.ignore; and lacks a real @Inject annotation in mate.academy.lib. You need a proper Inject annotation implementation in the correct package, not this placeholder.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
This file is just a placeholder with package mate.academy.lib.ignore; and no InjectionException class body in mate.academy.lib. Checklist item #7 requires a working custom exception with informative constructors in the proper package; this stub violates that requirement.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
This stubbed Injector.java only declares package mate.academy.lib.ignore; and does not contain the injector implementation in mate.academy.lib. The task requires a functioning Injector in the mate.academy.lib package; you should remove this stub or replace it with the full implementation and correct package name.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
This file is a placeholder with package mate.academy.lib.ignore; and no annotation definition. The task requires a working @Component annotation in the mate.academy.lib package. You should either remove this file/directory or replace it with the real Component implementation using package mate.academy.lib;.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
This file is a placeholder with package mate.academy.lib.ignore; and no annotation definition. The task requires a working @Inject annotation in the mate.academy.lib package. You should either remove this file/directory or replace it with the real Inject implementation using package mate.academy.lib;.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
This file is a placeholder with package mate.academy.lib.ignore; and no exception implementation. The task requires a working InjectionException in the mate.academy.lib package (with constructors that accept a message and a message+cause). You should either remove this file/directory or replace it with the real InjectionException implementation using package mate.academy.lib;.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
This file is a placeholder with package mate.academy.lib.ignore; and no injector implementation. The task requires a working Injector class in the mate.academy.lib package. You should either remove this file/directory or replace it with the real Injector implementation using package mate.academy.lib;.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
This file is a placeholder with package mate.academy.lib.ignore; and no annotation definition. This violates the high-priority requirement to have a working Component annotation in the mate.academy.lib package and conflicts with the real implementation under src/main/java/mate.academy/lib. You should remove this file or replace it with the real Component implementation using package mate.academy.lib;.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
Similarly, this placeholder Inject file in the wrong package (mate.academy.lib.ignore) conflicts with the requirement to have a single, working @Inject annotation in mate.academy.lib. It should be removed or replaced with the actual implementation using the correct package.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
This stub InjectionException in mate.academy.lib.ignore violates the requirement that InjectionException must exist and be implemented in the mate.academy.lib package (constructors with informative message and message+cause). Remove or replace it with the real implementation under the correct package.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
This placeholder Injector class in mate.academy.lib.ignore prevents you from having a single, authoritative Injector in the mate.academy.lib package as required. It should be removed, or your real Injector implementation should be moved here with package mate.academy.lib;.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
This file must either be removed or replaced with the real @Component implementation in the mate.academy.lib package; currently it only declares package mate.academy.lib.ignore; and violates the requirement to have a single, working mate.academy.lib package containing the DI annotations.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
This file must either be removed or replaced with the real @Inject implementation in the mate.academy.lib package; as-is it only declares package mate.academy.lib.ignore;, so the DI core is not consistently provided in mate.academy.lib.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
This placeholder InjectionException file with package mate.academy.lib.ignore; violates the requirement that the DI package mate.academy.lib contain a working InjectionException with informative constructors; either remove this file or replace it with the real implementation in the correct package.
| @@ -0,0 +1 @@ | |||
| package mate.academy.lib.ignore; | |||
There was a problem hiding this comment.
This placeholder Injector file with package mate.academy.lib.ignore; conflicts with the requirement to provide a functional Injector in the mate.academy.lib package; it should be removed or replaced by copying the working Injector implementation here with the correct package name.
No description provided.