Implement tasks #1760

euulu · 2024-12-11T13:59:08Z

No description provided.

mateacademy-ai-mentor

Thank you for your hard work on this pull request! 😊 While your implementation is mostly on the right track, there are a few critical issues that need to be addressed to meet the core requirements of the task:

Static Methods Usage: The use of static methods in FigureSupplier and ColorSupplier for getRandomFigure, getDefaultFigure, and getRandomColor is against the checklist requirements. These should be instance methods, and you should create an instance of FigureSupplier outside the loop in Application.java.
Magic Numbers: There are magic numbers in your code, particularly in FigureSupplier. These should be replaced with named constants to improve code readability and maintainability.
Enum String Representation: Ensure that you use name() instead of toString() for enum constants to align with best practices.

Addressing these issues will not only align your code with the task requirements but also enhance its quality and maintainability. Keep up the great work, and feel free to reach out if you have any questions or need further guidance! 🚀

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

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

mateacademy-ai-mentor · 2024-12-11T14:01:37Z

src/main/java/core/basesyntax/Application.java

+        Figure[] figures = new Figure[6];
+        int halfSize = figures.length / 2;
+        for (int i = 0; i < halfSize; i++) {
+            figures[i] = FigureSupplier.getRandomFigure();


Avoid using static methods for getRandomFigure and getDefaultFigure. Instead, create an instance of FigureSupplier and call these methods on that instance. This aligns with the checklist item: 'Don't use static methods in your solution'.

mateacademy-ai-mentor · 2024-12-11T14:01:37Z

src/main/java/core/basesyntax/Application.java

+        int halfSize = figures.length / 2;
+        for (int i = 0; i < halfSize; i++) {
+            figures[i] = FigureSupplier.getRandomFigure();
+            figures[halfSize + i] = FigureSupplier.getDefaultFigure();


Similarly, avoid using static methods for getDefaultFigure. Create an instance of FigureSupplier before the loop and use it to call getDefaultFigure. This aligns with the checklist item: 'You should create several random Figures, so you will use a loop. Please don't create a new FigureSupplier() inside the loop.'

mateacademy-ai-mentor · 2024-12-11T14:01:37Z

src/main/java/core/basesyntax/ColorSupplier.java

+import java.util.Random;
+
+public class ColorSupplier {
+    public static String getRandomColor() {


Avoid using a static method for getRandomColor. Convert this method to an instance method by removing the static keyword. This aligns with the checklist item: 'Don't use static methods in your solution'.

mateacademy-ai-mentor · 2024-12-11T14:01:38Z

src/main/java/core/basesyntax/FigureSupplier.java

+import java.util.Random;
+
+public class FigureSupplier {
+    public static Figure getRandomFigure() {


Avoid using a static method for getRandomFigure. Convert this method to an instance method by removing the static keyword. This aligns with the checklist item: 'Don't use static methods in your solution'.

mateacademy-ai-mentor · 2024-12-11T14:01:38Z

src/main/java/core/basesyntax/FigureSupplier.java

+        return randomFigure;
+    }
+
+    public static Figure getDefaultFigure() {


Avoid using a static method for getDefaultFigure. Convert this method to an instance method by removing the static keyword. This aligns with the checklist item: 'Don't use static methods in your solution'.

euulu · 2024-12-11T15:02:00Z

Enum String Representation: Ensure that you use name() instead of toString() for enum constants to align with best practices.

Can you please explain in detailsthis point from your review. Where exactly should i use name() method instead of toString()?

mateacademy-ai-mentor

Great job on implementing the solution! 🎉 Your code meets the task requirements, and you've made significant improvements based on the feedback. Here are some key points to consider for further enhancement:

Magic Numbers: You've addressed the issue of magic numbers by using constants for default values in FigureSupplier. However, remember to replace the number 6 used for the array size and the RGB values in DEFAULT_COLOR with named constants to improve readability and maintainability.
Static Methods: You've correctly avoided static methods by creating instances of FigureSupplier and ColorSupplier. This aligns well with the task requirements.
Area Calculation: Ensure the formula for calculating the area of an isosceles trapezoid is correct. The correct formula is ((bottom + top) / 2) * height, where height can be calculated using the Pythagorean theorem.
Random Values: Consider setting a realistic range for random values to ensure meaningful figures are generated.

Your efforts to address the feedback are commendable, and the decision to approve the pull request is to help you move forward with your learning. Please review the comments to further improve your code. Keep up the great work, and continue to learn from these experiences! 🚀

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

mateacademy-ai-mentor · 2024-12-11T15:04:22Z

src/main/java/core/basesyntax/Application.java

+    public static void main(String[] args) {
+        ColorSupplier colorSupplier = new ColorSupplier();
+        FigureSupplier figureSupplier = new FigureSupplier(colorSupplier);
+        Figure[] figures = new Figure[6];


Consider replacing the magic number 6 with a named constant to improve code readability and maintainability. This aligns with the checklist requirement to avoid magic numbers in your code.

mateacademy-ai-mentor · 2024-12-11T15:04:22Z

src/main/java/core/basesyntax/FigureSupplier.java

+
+public class FigureSupplier {
+    public static final String DEFAULT_COLOR = "rgb(255, 255, 255)";
+    public static final double DEFAULT_RADIUS = 10.0;


Consider replacing the magic number 10.0 with a named constant to improve code readability and maintainability. This aligns with the checklist requirement to avoid magic numbers in your code.

mateacademy-ai-mentor · 2024-12-11T15:04:22Z

src/main/java/core/basesyntax/FigureSupplier.java

+import java.util.Random;
+
+public class FigureSupplier {
+    public static final String DEFAULT_COLOR = "rgb(255, 255, 255)";


Consider replacing the RGB values in DEFAULT_COLOR with named constants to improve code readability and maintainability.

mateacademy-ai-mentor · 2024-12-11T15:04:23Z

src/main/java/core/basesyntax/FigureSupplier.java

+        double squareSide = random.nextDouble();
+        double width = random.nextDouble();
+        double height = random.nextDouble();
+        double firstLeg = random.nextDouble();
+        double secondLeg = random.nextDouble();
+        double radius = random.nextDouble();
+        double bottom = random.nextDouble();
+        double top = random.nextDouble();
+        double trapSide = random.nextDouble();


The random values for dimensions should be within a realistic range to ensure meaningful figures are generated. Consider setting a minimum and maximum value for these random numbers.

euulu added 8 commits December 11, 2024 15:56

Remove unneeded files

648d5f6

Add Figure class

e9a4fb8

Add Figures enum

34c0654

Add Drawable interface

914f263

Add ColorSupplier

97552aa

Add FigureSupplier

823960e

Add figure concrete instances

e8cef99

Use implemented functionality

be5e385

mateacademy-ai-mentor suggested changes Dec 11, 2024

View reviewed changes

euulu added 2 commits December 11, 2024 16:56

Refactor to use instance methods instead of static methods

0c2f03e

Refactor the line longer than 100 chars to shorter one

4819868

euulu requested a review from mateacademy-ai-mentor December 11, 2024 15:02

mateacademy-ai-mentor approved these changes Dec 11, 2024

View reviewed changes

euulu added 5 commits December 11, 2024 17:13

Refactor magic figures arr length into constant

1579066

Extract color values to an enum

2cdfd4b

Use Colors enum to define figure color

fd9ebc8

Change the imports order

aa4a06a

Use right IsoscelesTrapezoid area calculation formula

576dc56

euulu requested a review from mateacademy-ai-mentor December 11, 2024 15:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement tasks #1760

Implement tasks #1760

euulu commented Dec 11, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

euulu commented Dec 11, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

mateacademy-ai-mentor Dec 11, 2024

Implement tasks #1760

Are you sure you want to change the base?

Implement tasks #1760

Conversation

euulu commented Dec 11, 2024

mateacademy-ai-mentor 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

euulu commented Dec 11, 2024

mateacademy-ai-mentor 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