Done #1775

mizuo-san · 2025-01-02T18:13:57Z

No description provided.

mateacademy-ai-mentor

Thank you for your hard work on this project! While your code is well-structured and mostly meets the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:

Static Methods Usage: The use of static methods in FigureSupplier and ColorSupplier goes against the checklist's recommendations. It's important to create instances of these classes and call methods on those instances to adhere to best practices.
Isosceles Trapezoid Area Calculation: The formula for calculating the area of an isosceles trapezoid is incorrect. It should be ((firstBase + secondBase) / 2) * height. This is a critical issue as it affects the functionality of your code.
Magic Numbers: The presence of magic numbers, such as 4 in the FigureSupplier, should be replaced with named constants for better readability and maintainability.
Inheritance Misuse: Extending ColorSupplier in FigureSupplier is unnecessary and doesn't align with the task requirements. Consider using composition instead.

These issues are important to address to ensure your code is functional and maintainable. Once these changes are made, your code will be in great shape! Keep up the good work, and don't hesitate to reach out if you have any questions. 😊

✨ 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 · 2025-01-02T18:16:37Z

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

+
+import java.util.Random;
+
+public class FigureSupplier extends ColorSupplier {


Avoid extending ColorSupplier as it doesn't provide any additional benefit in this context. Consider using composition instead of inheritance if you need to use methods from ColorSupplier.

mateacademy-ai-mentor · 2025-01-02T18:16:38Z

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

+import java.util.Random;
+
+public class FigureSupplier extends ColorSupplier {
+    private final static Random picker = new Random();


Consider defining the number of figure types as a constant for better readability and maintainability. For example, private static final int FIGURE_TYPES_COUNT = 4;.

mateacademy-ai-mentor · 2025-01-02T18:16:38Z

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

+    public static Figure getRandomFigure() {
+        return switch(picker.nextInt(4)) {
+            case 0 -> new Circle(getRandomColor(), picker.nextInt(1, 50));
+            case 1 -> new Rectangle(getRandomColor(), picker.nextInt(1, 50), picker.nextInt(1, 50));
+            case 2 -> new RightTriangle(getRandomColor(), picker.nextInt(1, 50), picker.nextInt(1, 50));
+            case 3 -> new Square(getRandomColor(), picker.nextInt(1, 50));
+            default -> new Circle(Colors.WHITE, 10);
+        };
+    }


Avoid using static methods. Instead, create an instance of FigureSupplier and call the method on that instance. This aligns with the checklist's recommendation to avoid static methods.

mateacademy-ai-mentor · 2025-01-02T18:16:38Z

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

+        return switch(picker.nextInt(4)) {
+            case 0 -> new Circle(getRandomColor(), picker.nextInt(1, 50));
+            case 1 -> new Rectangle(getRandomColor(), picker.nextInt(1, 50), picker.nextInt(1, 50));
+            case 2 -> new RightTriangle(getRandomColor(), picker.nextInt(1, 50), picker.nextInt(1, 50));
+            case 3 -> new Square(getRandomColor(), picker.nextInt(1, 50));
+            default -> new Circle(Colors.WHITE, 10);
+        };


Replace the magic number 4 with a constant, such as FIGURE_TYPES_COUNT, to improve code readability and maintainability.

mateacademy-ai-mentor · 2025-01-02T18:16:38Z

src/main/java/core/basesyntax/IsoscelesTrapezoid.java

+    public double area() {
+        return (firstBase + secondBase) * height;


The formula for the area of an isosceles trapezoid is incorrect. It should be ((firstBase + secondBase) / 2) * height to correctly calculate the area.

mateacademy-ai-mentor · 2025-01-02T18:16:38Z

src/main/java/core/basesyntax/Main.java

+package core.basesyntax;
+
+public class Main {
+    public static void main(String args[]) {


It's a common convention to declare the main method parameter as String[] args instead of String args[]. Consider changing it for consistency.

mateacademy-ai-mentor · 2025-01-02T18:16:38Z

src/main/java/core/basesyntax/Main.java

+    public static Figure[] listOfRandomFigures() {
+        Figure[] array = new Figure[6];
+
+        for(int i = 0; i < 3; i++) {
+            array[i] = FigureSupplier.getRandomFigure();
+        }
+
+        for(int i = 3; i < 6; i++) {
+            array[i] = FigureSupplier.getDefaultFigure();
+        }
+
+        return array;
+    }
+
+    public static void printArray() {
+        for(Figure i : listOfRandomFigures()) {
+            i.draw();
+        }
+    }


Avoid using static methods for listOfRandomFigures and printArray. Consider creating an instance of Main and calling these methods on that instance, aligning with the checklist's recommendation to avoid static methods.

mateacademy-ai-mentor

Review Summary

Great job on implementing the various classes and addressing most of the previous review comments! Your code is well-structured and adheres to the task requirements. Here are some key points and suggestions for further improvement:

Static Methods Usage: You've correctly moved away from static methods by creating instances of FigureSupplier and ColorSupplier. This is a good practice for object-oriented programming.
Isosceles Trapezoid Area Calculation: The formula for calculating the area is now correct, ensuring accurate results.
Magic Numbers: You've improved readability by replacing the magic number 4 with a named constant. This makes your code more understandable and maintainable.
Inheritance Misuse: The unnecessary inheritance from ColorSupplier in FigureSupplier has been removed, aligning with proper design principles.
Switch Statement Default Case: Although the default case in your switch statement is redundant due to the range of picker.nextInt(randomNum), it's a minor issue. You can safely remove it for cleaner code.

Overall, your code meets the task requirements and is ready for further use. Remember to address any remaining issues in future projects to enhance your coding skills. Keep up the great work, and continue learning from these experiences! 🚀

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

mateacademy-ai-mentor · 2025-01-02T18:50:33Z

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

+
+public class FigureSupplier {
+    private final Random picker = new Random();
+    private final int randomNum = 4;


The use of the magic number 4 for randomNum should be replaced with a named constant to improve code readability and maintainability. Consider defining a constant like private static final int NUMBER_OF_FIGURES = 4;.

mateacademy-ai-mentor · 2025-01-02T18:50:33Z

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

+                    picker.nextInt(1, 50));
+            case 3 -> new Square(colorSupplier.getRandomColor(),
+                    picker.nextInt(1, 50));
+            default -> new Circle(Colors.WHITE, 10);


The default case in the switch statement is redundant because the picker.nextInt(randomNum) will always return a value between 0 and 3. You can safely remove the default case.

liliia-ponomarenko

Let’s improve your solution ;)

liliia-ponomarenko · 2025-01-03T08:53:22Z

src/main/java/core/basesyntax/Colors.java

@@ -0,0 +1,9 @@
+package core.basesyntax;
+
+public enum Colors {


Suggested change

public enum Colors {

public enum Color {

liliia-ponomarenko · 2025-01-03T08:54:02Z

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

+
+public class FigureSupplier {
+    private final Random picker = new Random();
+    private final int randomNum = 4;


it should be a constant

liliia-ponomarenko · 2025-01-03T08:55:38Z

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

+                    picker.nextInt(1, 50));
+            case 3 -> new Square(colorSupplier.getRandomColor(),
+                    picker.nextInt(1, 50));
+            default -> new Circle(Colors.WHITE, 10);


IsoscelesTrapesoid missed

liliia-ponomarenko · 2025-01-03T08:56:00Z

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

+                    picker.nextInt(1, 50));
+            case 1 -> new Rectangle(colorSupplier.getRandomColor(),
+                    picker.nextInt(1, 50),
+                    picker.nextInt(1, 50));
+            case 2 -> new RightTriangle(colorSupplier.getRandomColor(),
+                    picker.nextInt(1, 50),
+                    picker.nextInt(1, 50));
+            case 3 -> new Square(colorSupplier.getRandomColor(),
+                    picker.nextInt(1, 50));


Make numbers constant field

liliia-ponomarenko · 2025-01-03T08:56:13Z

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

+    }
+
+    public Figure getDefaultFigure() {
+        return new Circle(Colors.WHITE, 10);


liliia-ponomarenko · 2025-01-03T08:56:50Z

src/main/java/core/basesyntax/Main.java

+    public static Figure[] listOfRandomFigures() {
+        Figure[] array = new Figure[6];
+
+        for (int i = 0; i < 3; i++) {
+            array[i] = figureSupplier.getRandomFigure();
+        }
+
+        for (int i = 3; i < 6; i++) {
+            array[i] = figureSupplier.getDefaultFigure();
+        }
+
+        return array;
+    }
+
+    public static void printArray() {
+        for (Figure i : listOfRandomFigures()) {
+            i.draw();
+        }
+    }


these methods can be private

liliia-ponomarenko · 2025-01-03T08:57:19Z

src/main/java/core/basesyntax/Main.java

+    }
+
+    public static void printArray() {
+        for (Figure i : listOfRandomFigures()) {


create local variable for listOfRandomFigures()

Elena-Bruyako · 2025-01-04T11:25:54Z

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

+
+import java.util.Random;
+
+public final class ColorSupplier {


Suggested change

public final class ColorSupplier {

public class ColorSupplier {

Elena-Bruyako · 2025-01-04T11:26:02Z

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

+public final class ColorSupplier {
+    private final Random picker = new Random();
+
+    public Color getRandomColor() {


Suggested change

public Color getRandomColor() {

public String getRandomColor() {

Elena-Bruyako · 2025-01-04T11:26:09Z

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

+import java.util.Random;
+
+public final class ColorSupplier {
+    private final Random picker = new Random();


Suggested change

private final Random picker = new Random();

private final Random random = new Random();

Elena-Bruyako · 2025-01-04T11:26:16Z

src/main/java/core/basesyntax/Circle.java

+    private final float radius;
+
+    public Circle(Color color, float radius) {
+        super(color, "Circle");


Suggested change

super(color, "Circle");

super(color);

Elena-Bruyako · 2025-01-04T11:26:21Z

src/main/java/core/basesyntax/Circle.java

+public class Circle extends Figure {
+    private final float radius;
+
+    public Circle(Color color, float radius) {


Suggested change

public Circle(Color color, float radius) {

public Circle(String color, float radius) {

Elena-Bruyako · 2025-01-04T11:28:48Z

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

+    public Figure getRandomFigure() {
+        final int figureCount = 5;
+        final int startNum = 1;
+        final int limitNum = 50;


Suggested change

final int limitNum = 50;

final int limitNum = 50;

String randomColor = colorSupplier.getRandomColor();

Elena-Bruyako · 2025-01-04T11:29:00Z

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

+        final int limitNum = 50;
+
+        return switch (picker.nextInt(figureCount)) {
+            case 0 -> new Circle(colorSupplier.getRandomColor(),


same for other figures

Suggested change

case 0 -> new Circle(colorSupplier.getRandomColor(),

case 0 -> new Circle(randomColor,

Elena-Bruyako · 2025-01-04T11:29:25Z

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

+
+        return switch (picker.nextInt(figureCount)) {
+            case 0 -> new Circle(colorSupplier.getRandomColor(),
+                    picker.nextInt(startNum, limitNum));


create method separate method to get picker.nextInt(startNum, limitNum)); and reuse it

Elena-Bruyako · 2025-01-04T11:29:34Z

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

+                    picker.nextInt(startNum, limitNum),
+                    picker.nextInt(startNum, limitNum),
+                    picker.nextInt(startNum, limitNum));
+            default -> new Circle(Color.WHITE, 10);


10 should be constant

Elena-Bruyako · 2025-01-04T11:34:49Z

src/main/java/core/basesyntax/Main.java

+    private static final FigureSupplier figureSupplier = new FigureSupplier();
+
+    public static void main(String[] args) {
+        printArray();


redundant separate method, create Figure[] here, 6 - should be constant, use one for loop for this task

mizuo-san added 3 commits January 2, 2025 18:34

bober

5f8db2f

bober

c26debc

done?

00af0f2

mateacademy-ai-mentor suggested changes Jan 2, 2025

View reviewed changes

mizuo-san added 8 commits January 2, 2025 19:16

checksyntax

f453c23

checksyntax x2

5592940

checksyntax x3

1961220

checksyntax x4

b72ffb0

checksyntax x5

2a607bb

checksyntax x6

bb7e4f2

definetly not a checkstyle

f57a1bf

icantanymore

022b7d6

mizuo-san requested a review from mateacademy-ai-mentor January 2, 2025 18:48

mateacademy-ai-mentor approved these changes Jan 2, 2025

View reviewed changes

liliia-ponomarenko suggested changes Jan 3, 2025

View reviewed changes

added IsoscelesTrapezoid figure in random and minor fixes

e198d0b

mizuo-san requested a review from liliia-ponomarenko January 3, 2025 15:43

make suppliers final

9b0023b

Elena-Bruyako suggested changes Jan 4, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Done #1775

Done #1775

mizuo-san commented Jan 2, 2025

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Jan 2, 2025

mateacademy-ai-mentor Jan 2, 2025

mateacademy-ai-mentor Jan 2, 2025

mateacademy-ai-mentor Jan 2, 2025

mateacademy-ai-mentor Jan 2, 2025

mateacademy-ai-mentor Jan 2, 2025

mateacademy-ai-mentor Jan 2, 2025

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Jan 2, 2025

mateacademy-ai-mentor Jan 2, 2025

liliia-ponomarenko left a comment

liliia-ponomarenko Jan 3, 2025

liliia-ponomarenko Jan 3, 2025

mizuo-san Jan 3, 2025

liliia-ponomarenko Jan 3, 2025

liliia-ponomarenko Jan 3, 2025

liliia-ponomarenko Jan 3, 2025

liliia-ponomarenko Jan 3, 2025

liliia-ponomarenko Jan 3, 2025

Elena-Bruyako Jan 4, 2025

Elena-Bruyako Jan 4, 2025

Elena-Bruyako Jan 4, 2025

Elena-Bruyako Jan 4, 2025

Elena-Bruyako Jan 4, 2025

Elena-Bruyako Jan 4, 2025

Elena-Bruyako Jan 4, 2025

Elena-Bruyako Jan 4, 2025

Elena-Bruyako Jan 4, 2025

Elena-Bruyako Jan 4, 2025


		import java.util.Random;

		public class FigureSupplier extends ColorSupplier {

		public double area() {
		return (firstBase + secondBase) * height;

	public final class ColorSupplier {
	public class ColorSupplier {

	public Color getRandomColor() {
	public String getRandomColor() {

	private final Random picker = new Random();
	private final Random random = new Random();

	public Circle(Color color, float radius) {
	public Circle(String color, float radius) {

	final int limitNum = 50;
	final int limitNum = 50;
	String randomColor = colorSupplier.getRandomColor();

	case 0 -> new Circle(colorSupplier.getRandomColor(),
	case 0 -> new Circle(randomColor,

Done #1775

Are you sure you want to change the base?

Done #1775

Conversation

mizuo-san commented Jan 2, 2025

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liliia-ponomarenko 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

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

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

Choose a reason for hiding this comment