feat(DEVELOP): create Figure structure #1951

Gosyav · 2025-09-02T16:04:59Z

No description provided.

mateacademy-ai-mentor

Review Summary

Decision: REQUEST_CHANGES — there are a few critical requirement violations that block full compliance.

Blocking issues to fix:

draw() outputs missing units (requirements 1.5, 3.8)
- Files: Circle.java (e.g., radius/area), Square.java (side/area), Rectangle.java (width/height/area), RightTriangle.java (base/height/area), IsoscelesTrapezoid.java (base/height/side/area)
- Why: The task requires each dimension and area to be printed with “units”. Missing units makes the output non-compliant and can cause test failures.
- How: Update each draw() to include units, e.g., radius: <value> units, area: <value> units.
ColorSupplier return type (requirement 1.6)
- File: src/main/java/core/basesyntax/service/ColorSupplier.java, method getRandomColor()
- Why: Must return a random color string, not a Color enum.
- How: Change signature to return String and return colors[randomIndex].name().
FigureSupplier collaborator scope and magic number (checklist 6, 7)
- File: src/main/java/core/basesyntax/service/FigureSupplier.java
- Why: Instantiating ColorSupplier inside getRandomFigure() and using 5 directly both violate checklist requirements and can lead to inefficiency and brittle code.
- How: Make ColorSupplier a field initialized in the class. Replace 5 in random.nextInt(5) + 1 with a named constant like FIGURE_TYPES_COUNT.
Enum string usage and default case handling (checklist 12, 14)
- File: FigureSupplier.java
- Why: If getRandomColor() returns Color, use .name(); if returning String, remove .toString(). The switch default should return a valid figure to ensure the method always returns a figure.
- How: Adjust color assignment accordingly. In the switch default, return a valid default figure instead of throwing a RuntimeException.

Minor notes:

Ensure all places that output colors use name() if working with enums.

You’ve done a solid job on structure and main flow. With these targeted fixes, your solution should fully meet the requirements. Keep it up!

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2025-09-02T16:10:24Z

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

+        System.out.println("Circle: "
+                + "color: " + getColor()
+                + ", radius: " + getRadius()
+                + ", area: " + getArea());


The draw() output does not include units for the radius or area, as required by the task description and checklist (requirement 1.5, 3.8: "Each dimension with units 'units'"). Consider updating the output to include units, e.g., radius: X units, area: Y units.

mateacademy-ai-mentor · 2025-09-02T16:10:25Z

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

+                + ", base: " + getBase()
+                + ", height: " + getHeight()
+                + ", side: " + getSide()
+                + ", area: " + getArea());


The draw() output does not include units for the base, height, side, or area, as required by the task description and checklist (requirement 1.5, 3.8: "Each dimension with units 'units'"). Consider updating the output to include units, e.g., base: X units, height: Y units, side: Z units, area: Y units.

mateacademy-ai-mentor · 2025-09-02T16:10:25Z

src/main/java/core/basesyntax/model/Rectangle.java

+                + "color: " + getColor()
+                + ", width: " + getWidth()
+                + ", height: " + getHeight()
+                + ", area: " + getArea());


The draw() output does not include units for the width, height, or area, as required by the task description and checklist (requirement 1.5, 3.8: "Each dimension with units 'units'"). Consider updating the output to include units, e.g., width: X units, height: Y units, area: Y units.

mateacademy-ai-mentor · 2025-09-02T16:10:25Z

src/main/java/core/basesyntax/model/RightTriangle.java

+                + "color: " + getColor()
+                + ", base: " + getFirstLeg()
+                + ", height: " + getSecondLeg()
+                + ", area: " + getArea());


The draw() output does not include units for the base, height, or area, as required by the task description and checklist (requirement 1.5, 3.8: "Each dimension with units 'units'"). Consider updating the output to include units, e.g., base: X units, height: Y units, area: Y units.

mateacademy-ai-mentor · 2025-09-02T16:10:25Z

src/main/java/core/basesyntax/model/Square.java

+        System.out.println("Square: "
+                + "color: " + getColor()
+                + ", side: " + getSide()
+                + ", area: " + getArea());


The draw() output does not include units for the side or area, as required by the task description and checklist (requirement 1.5, 3.8: "Each dimension with units 'units'"). Consider updating the output to include units, e.g., side: X units, area: Y units.

mateacademy-ai-mentor · 2025-09-02T16:10:25Z

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

+    public Color getRandomColor() {
+        int randomIndex = random.nextInt(colors.length);
+
+        return colors[randomIndex];


The method getRandomColor() returns a Color enum, but the requirements specify it should return a String (requirement 1.6: "returns a random color string"). Consider changing the return type to String and returning colors[randomIndex].name().

mateacademy-ai-mentor · 2025-09-02T16:10:25Z

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

+    private Random random = new Random();
+
+    public Figure getRandomFigure() {
+        ColorSupplier colorSupplier = new ColorSupplier();


The ColorSupplier is instantiated inside getRandomFigure(). According to checklist item 6, persistent collaborators like ColorSupplier should be fields, not local variables. Move ColorSupplier to a field in the class.

mateacademy-ai-mentor · 2025-09-02T16:10:25Z

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

+
+    public Figure getRandomFigure() {
+        ColorSupplier colorSupplier = new ColorSupplier();
+        int randomIndex = random.nextInt(5) + 1;


The number 5 in random.nextInt(5) + 1 is a magic number. According to checklist item 7, use a named constant for the number of figure types instead of a magic number.

mateacademy-ai-mentor · 2025-09-02T16:10:25Z

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

+    public Figure getRandomFigure() {
+        ColorSupplier colorSupplier = new ColorSupplier();
+        int randomIndex = random.nextInt(5) + 1;
+        String color = colorSupplier.getRandomColor().toString();


If getRandomColor() returns a Color enum, you should use .name() instead of .toString() when assigning to color, as per checklist item 14. If it returns a String, .toString() is redundant and should be removed.

mateacademy-ai-mentor · 2025-09-02T16:10:26Z

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

+                side = random.nextInt(10) + 1;
+                return new core.basesyntax.model.IsoscelesTrapezoid(base, height, side, color);
+            default:
+                throw new RuntimeException("Can't create figure");


In the switch's default case, you throw a RuntimeException. Checklist item 12 states: "Do not return null from methods like getRandomFigure(), getDefaultFigure(), or any switch-based factory logic. Ensure a valid Figure is always returned, using default case if switch is used." Instead of throwing, return a valid default figure.

mateacademy-ai-mentor

Review Summary

Decision: Request changes because output formatting violates explicit requirements and will likely fail tests.

Critical issues to fix (blockers):

Area units must be exactly "sq. units" in all draw() outputs.
- Files and suggested fixes:
  - src/main/java/core/basesyntax/model/Circle.java: In draw() (line with area printing), append " sq. units" after getArea().
    - Example: ", area: " + getArea() + " sq. units"
  - src/main/java/core/basesyntax/model/IsoscelesTrapezoid.java: Replace "square units" with "sq. units" in draw().
  - src/main/java/core/basesyntax/model/Rectangle.java: Replace "square units" with "sq. units" in draw().
  - src/main/java/core/basesyntax/model/RightTriangle.java: Replace "square units" with "sq. units" in draw().
  - src/main/java/core/basesyntax/model/Square.java: Replace "square units" with "sq. units" in draw().
- Why: The spec requires the exact format area: <value> sq. units. Mismatches will break output-based tests.
RightTriangle color output format is incorrect.
- File: src/main/java/core/basesyntax/model/RightTriangle.java
- Issue: draw() appends " units" to the color (e.g., color: yellow units).
- Fix: Print only the color string: "color: " + getColor() without extra text.

Minor (non-blocking) note:

FigureSupplier uses 1-based random switch index. It’s fine, but 0-based is more idiomatic.

You’ve done a solid job with class structure, area calculations, and suppliers. Once you align the output strings with the exact required format, your solution should fully pass. Keep it up!

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2025-09-02T16:31:55Z

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

+        System.out.println("Circle: "
+                + "color: " + getColor()
+                + ", radius: " + getRadius() + " units"
+                + ", area: " + getArea());


The area output in draw() is missing the required units. It should be:
+ ", area: " + getArea() + " sq. units"
This violates the requirement that area must be printed with units (e.g., 'area: 25.0 sq. units').

mateacademy-ai-mentor · 2025-09-02T16:31:55Z

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

+                + ", base: " + getBase() + " units"
+                + ", height: " + getHeight() + " units"
+                + ", side: " + getSide() + " units"
+                + ", area: " + getArea() + " square units");


The area output in draw() should use "sq. units" instead of "square units" to match the required format: 'area: sq. units'.

mateacademy-ai-mentor · 2025-09-02T16:31:55Z

src/main/java/core/basesyntax/model/Rectangle.java

+                + "color: " + getColor()
+                + ", width: " + getWidth() + " units"
+                + ", height: " + getHeight() + " units"
+                + ", area: " + getArea() + " square units");


The area output in draw() should use "sq. units" instead of "square units" to match the required format: 'area: sq. units'.

mateacademy-ai-mentor · 2025-09-02T16:31:55Z

src/main/java/core/basesyntax/model/RightTriangle.java

+    @Override
+    public void draw() {
+        System.out.println("RightTriangle: "
+                + "color: " + getColor() + " units"


The color output in draw() incorrectly appends " units" to the color. It should be just the color string, e.g., 'color: yellow'.

mateacademy-ai-mentor · 2025-09-02T16:31:55Z

src/main/java/core/basesyntax/model/RightTriangle.java

+                + "color: " + getColor() + " units"
+                + ", base: " + getFirstLeg() + " units"
+                + ", height: " + getSecondLeg() + " units"
+                + ", area: " + getArea() + " square units");


The area output in draw() should use "sq. units" instead of "square units" to match the required format: 'area: sq. units'.

mateacademy-ai-mentor · 2025-09-02T16:31:56Z

src/main/java/core/basesyntax/model/Square.java

+        System.out.println("Square: "
+                + "color: " + getColor()
+                + ", side: " + getSide() + " units"
+                + ", area: " + getArea() + " square units");


The area output in draw() should use "sq. units" instead of "square units" to match the required format: 'area: sq. units'.

mateacademy-ai-mentor

Review Summary

Educational approval: Great progress overall—your design cleanly follows OOP principles, suppliers are used via composition, and Main correctly creates and draws figures. Nice job keeping constants, avoiding static business logic, and ensuring no nulls are returned.

However, there’s a small output-format issue that can break tests:

src/main/java/core/basesyntax/model/Circle.java: In draw() (likely around the line where you print area), you print "area: ... square units" but the requirement is exactly "area: ... sq. units".
src/main/java/core/basesyntax/model/Rectangle.java: Same as above in draw().

Why this matters: tests compare exact strings; any deviation like "square units" won’t match. How to fix: change the hardcoded unit substring to "sq. units" in both draw() methods while keeping the rest of the format unchanged.

Everything else looks solid—area calculations, constructors, and the rest of the draw outputs are correct. Make that quick tweak and you should be all set. Keep up the great work!

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

Rate AI review example ↩

mateacademy-ai-mentor · 2025-09-02T16:43:12Z

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

+        System.out.println("Circle: "
+                + "color: " + getColor()
+                + ", radius: " + getRadius() + " units"
+                + ", area: " + getArea() + " square units");


This violates the requirement for exact output format: area must be printed as "area: ... sq. units" (not "square units").

See task description and previous review: output must be exactly "sq. units" to pass tests.

mateacademy-ai-mentor · 2025-09-02T16:43:12Z

src/main/java/core/basesyntax/model/Rectangle.java

+                + "color: " + getColor()
+                + ", width: " + getWidth() + " units"
+                + ", height: " + getHeight() + " units"
+                + ", area: " + getArea() + " square units");


This violates the requirement for exact output format: area must be printed as "area: ... sq. units" (not "square units").

See task description and previous review: output must be exactly "sq. units" to pass tests.

feat(DEVELOP): create Figure structure

1695b43

mateacademy-ai-mentor suggested changes Sep 2, 2025

View reviewed changes

fix(DEVELOP): fix

57fb6ca

Gosyav requested a review from mateacademy-ai-mentor September 2, 2025 16:26

mateacademy-ai-mentor suggested changes Sep 2, 2025

View reviewed changes

fix(DEVELOP): fix

c92f150

Gosyav requested a review from mateacademy-ai-mentor September 2, 2025 16:38

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

View reviewed changes

feat(DEVELOP): create Figure structure #1951

Are you sure you want to change the base?

feat(DEVELOP): create Figure structure #1951

Uh oh!

Conversation

Gosyav commented Sep 2, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants