Skip to content
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 #1015

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

rus1980707
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Congratulations on implementing the code according to the specifications! 🌟 Your use of annotations, exception handling, and adherence to dependency injection principles is commendable. Keep up the good work and always aim to handle potential exceptions gracefully to enhance the robustness of your applications. Happy coding! 💻🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨


@Retention(RetentionPolicy.RUNTIME)
public @interface Component {

Choose a reason for hiding this comment

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

According to the checklist, don't forget to add @Target to custom annotations. It specifies the kinds of program element to which an annotation type is applicable.

Comment on lines +50 to +51
} catch (Exception e) {
throw new RuntimeException("Failed to create instance of " + implClass.getName(), e);

Choose a reason for hiding this comment

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

It is better to replace many exceptions that have a common parent with a general parental exception to simplify the catch block. Consider using ReflectiveOperationException which is a common superclass of the exceptions thrown by reflective operations in the java.lang.reflect package.

Comment on lines 15 to 34
public Object getInstance(Class<?> interfaceClazz) {
return null;
// Try to resolve implementation for the given interface
Class<?> implClass = findImplementation(interfaceClazz);

// Check if class is annotated with @Component
if (!implClass.isAnnotationPresent(Component.class)) {
throw new RuntimeException("Class " + implClass.getName()
+ " is not marked as @Component");
}

// Check if an instance already exists
if (instances.containsKey(implClass)) {
return instances.get(implClass);
}

// Create a new instance and inject dependencies
Object instance = createInstance(implClass);
instances.put(implClass, instance);
return instance;
}

Choose a reason for hiding this comment

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

Make Interface Implementations map a class field. You can fill it in using Map.of(). This will help to avoid using the if statements in the findImplementation method for determining the implementation class.

Comment on lines +20 to +22
if (!implClass.isAnnotationPresent(Component.class)) {
throw new RuntimeException("Class " + implClass.getName()
+ " is not marked as @Component");

Choose a reason for hiding this comment

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

When throwing an exception add an informative message to it. Also, don't forget to add an exception you're catching in catch block to the RuntimeException object you throw. This will provide more context in the error message and make debugging easier.

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