Skip to content

Conversation

vfed45
Copy link

@vfed45 vfed45 commented Sep 27, 2025

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.

Review Summary

I’m requesting changes because there are critical blockers that affect correctness and required output formatting.

Critical issues to fix:

  • IsoscelesTrapezoid.getArea(): incorrect formula. Requirement is correct area computation. Replace with (upperBase + lowerBase) / 2.0 * height (file: src/main/java/core/basesyntax/IsoscelesTrapezoid.java, method getArea()). This directly impacts program correctness.
  • draw() output formatting must match the sample structure. Current prints will likely fail tests due to spacing/punctuation:
    • Circle.draw and Square.draw: add a space before sq. units (e.g., ... area: " + getArea() + " sq. units ...).
    • Rectangle.draw: add the missing colon after height and ensure the space before sq. units (e.g., ..., width: " + width + " units, height: " + height + " units, color: " + color).
  • FigureSupplier: replace magic number random.nextInt(5) with a constant (e.g., FIGURE_COUNT) and stop passing inline expressions to constructors. Assign random values to local variables first, then pass them. Also, move default values to constants (e.g., DEFAULT_COLOR = "white", DEFAULT_RADIUS = 10). These are explicit checklist requirements that the task enforces.

Additional checklist fixes (minor but required):

  • Remove redundant empty lines after package declarations and before members/methods in multiple files: AreaCalculatable.java, Circle.java, ColorSupplier.java, Drawable.java, Figure.java, IsoscelesTrapezoid.java, Main.java, Rectangle.java, RightTriangle.java, Square.java.
  • Main: replace the magic number for array size with a named constant (e.g., private static final int ARRAY_SIZE = 6).

Once you correct the area formula, normalize the draw() outputs, and address the constants/inline-expression issues in FigureSupplier (plus the small style fixes), the solution should meet all core requirements. Good job on the overall structure and separation of concerns—these are quick, focused changes to reach full compliance.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

@@ -0,0 +1,5 @@
package core.basesyntax;

Choose a reason for hiding this comment

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

This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove the empty line between the package statement and the interface declaration so the file begins directly with the interface definition.

@@ -0,0 +1,19 @@
package core.basesyntax;

Choose a reason for hiding this comment

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

This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" There is an unnecessary empty line immediately after the package declaration. Remove this empty line so the class starts directly after the package statement.

super(color);
this.radius = radius;
}

Choose a reason for hiding this comment

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

This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove the redundant empty line inside the class here (no need for a blank line).

public double getArea() {
return Math.PI * radius * radius;
}

Choose a reason for hiding this comment

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

This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove this redundant empty line as well to keep the class compact and follow the style guidance.


@Override
public void draw() {
System.out.println("Figure: circle, area: " + getArea() + "sq. units, radius: " + radius + " units, color: " + color);

Choose a reason for hiding this comment

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

This violates the requirement for draw() output formatting: "The draw() output should follow the sample structure: - Include figure name in lowercase, “area: {value} sq. units” - Include figure-specific fields with “units” - Include “color: {color}” ... While exact punctuation may vary, all listed elements must be present and printed via System.out.println() (no toString override)." Current output misses a space before "sq. units" (it prints "...area: {value}sq. units..."). Change the line to include that space so it becomes: System.out.println("Figure: circle, area: " + getArea() + " sq. units, radius: " + radius + " units, color: " + color);

public double getArea() {
return (firstLeg * secondLeg) / 2.0;
}

Choose a reason for hiding this comment

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

This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove the redundant empty line here (before the @OverRide of draw) to match the project's style guidance.

@@ -0,0 +1,19 @@
package core.basesyntax;

Choose a reason for hiding this comment

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

This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove the unnecessary empty line immediately after the package declaration so the class starts directly after the package statement.

super(color);
this.side = side;
}

Choose a reason for hiding this comment

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

This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove the redundant empty line inside the class (before the constructor) to follow the style guidance.

public double getArea() {
return side * side;
}

Choose a reason for hiding this comment

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

This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove the redundant empty line before the getArea() method to keep the class compact and consistent with the checklist.


@Override
public void draw() {
System.out.println("Figure: square, area: " + getArea() + "sq. units, side: " + side + " units, color: " + color);

Choose a reason for hiding this comment

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

This violates the requirement about draw() output formatting: "The draw() output should follow the sample structure: - Include figure name in lowercase, “area: {value} sq. units” - Include figure-specific fields with “units” - Include “color: {color}” While exact punctuation may vary, all listed elements must be present and printed via System.out.println() (no toString override)." Current print statement misses a space before "sq. units" (it prints "area: {value}sq. units"). Update the line to include the space and the exact labels, for example:
System.out.println("Figure: square, area: " + getArea() + " sq. units, side: " + side + " units, color: " + color);

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.

Review Summary

I’m requesting changes because there are explicit checklist violations and one output-format issue that can break tests. Functionality is largely correct, but the following must be fixed to meet required standards:

  • Critical/blocking: In Rectangle.draw() the label for height is missing a space, producing output like "height:5 units" instead of "height: 5 units". This can cause exact-output tests to fail. Fix by changing to "height: " + height.
  • Critical/checklist: Multiple files begin classes/methods with redundant empty lines (after package and between members). The checklist explicitly forbids this. Remove those blank lines in: AreaCalculatable.java, Circle.java, ColorSupplier.java, Drawable.java, Figure.java, FigureSupplier.java, IsoscelesTrapezoid.java, Main.java, Rectangle.java, RightTriangle.java, Square.java. Example lines to remove include the first blank line after each package declaration and blank lines before constructors/methods noted in comments.
  • Critical/checklist: Interface naming – AreaCalculatable should be a noun-style name (e.g., AreaCalculator). Rename the interface and update all implementors/usages.

Minor but recommended: replace the inline +1 in FigureSupplier with a constant (e.g., MIN_SIZE = 1), and consider making the colors array in ColorSupplier a static final constant. Overall, great job on formulas and separation of concerns—after these fixes, the solution should pass cleanly.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

@@ -0,0 +1,5 @@
package core.basesyntax;

Choose a reason for hiding this comment

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

There is a blank line immediately after the package declaration. The checklist explicitly requires: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Please remove the extra empty line so the interface declaration follows the package statement directly.

@@ -0,0 +1,5 @@
package core.basesyntax;

public interface AreaCalculatable {

Choose a reason for hiding this comment

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

The interface name AreaCalculatable violates the checklist guidance: "Don't use verbs for class/interface names". The checklist suggests using a noun-style name (example: AreaCalculator). Consider renaming this interface to AreaCalculator (or another noun) and update all implementors accordingly to match the naming requirement.

package core.basesyntax;

public interface AreaCalculatable {
double getArea();

Choose a reason for hiding this comment

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

The interface is functionally correct (declares double getArea()), which satisfies the separate-area-interface requirement. No change needed to the method signature itself. See AreaCalculatable.java source for reference .

@@ -0,0 +1,21 @@
package core.basesyntax;

Choose a reason for hiding this comment

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

This file contains a blank line immediately after the package declaration. This violates the checklist: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove the empty line after the package statement so the class declaration follows the package directly.


public class Circle extends Figure {
private double radius;

Choose a reason for hiding this comment

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

There is an empty line before the constructor declaration. This violates the checklist: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove this blank line so the constructor starts without a preceding empty line.

public double getArea() {
return (firstLeg * secondLeg) / 2.0;
}

Choose a reason for hiding this comment

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

There is a redundant empty line between the getArea() method and the draw() override. This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove the blank line at this location.

@@ -0,0 +1,21 @@
package core.basesyntax;

Choose a reason for hiding this comment

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

This file has an empty line immediately after the package declaration. This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove the blank line after the package statement.


public class Square extends Figure {
private double side;

Choose a reason for hiding this comment

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

There is a redundant empty line between the field declaration and the constructor. This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove the blank line at this location.

super(color);
this.side = side;
}

Choose a reason for hiding this comment

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

There is a redundant empty line between the constructor and the getArea() method annotation. This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove the blank line at this location.

public double getArea() {
return side * side;
}

Choose a reason for hiding this comment

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

There is a redundant empty line between the getArea() method and the draw() method annotation. This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove the blank line at this location.

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.

Review Summary

Educational approval: Your solution is functionally solid and meets the core requirements. Figures correctly compute areas and print details via draw(), suppliers are used properly, and constants/random generation are well organized.

What you did well:

  • Clear OOP design: Figure holds shared state; Drawable and AreaCalculator separate concerns.
  • Correct area formulas and informative draw() outputs in English across all shapes.
  • Main and FigureSupplier logic follows the checklist (single supplier instance, half random/half default, constants for config).

What to improve (style, required by checklist/Checkstyle):

  • Remove leading/redundant empty lines before constructors/methods. Examples:
    • Circle.java: remove blank lines before the constructor (line 4), before getArea() (line 9), and before draw() (line 14).
    • ColorSupplier.java: remove the empty line before the class, and the one between fields and the method; also remove the trailing blank line after the closing brace.
    • Figure.java: remove the empty lines before getArea() (line 7) and draw() (line 10).
    • FigureSupplier.java: remove redundant blank lines around constants, fields, local variable block, and before switch/getDefaultFigure().
    • IsoscelesTrapezoid.java, Rectangle.java, RightTriangle.java, Square.java, Main.java: remove the indicated blank lines before constructors/getArea()/draw() and within class bodies (see inline comments).
  • Why: This violates the checklist item “Don’t begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)”. It also triggers Checkstyle EmptyLineSeparator errors that block CI.
  • How: Keep exactly one blank line between package and imports, and between imports and class. Inside classes, avoid a blank line immediately before a constructor or method; group members logically without leading empty lines.

Minor but important fix from CI logs:

  • ColorSupplier.java: reorder fields to satisfy DeclarationOrder. Place static constants before instance fields, e.g. private static final String[] COLORS above private Random random.

You’re very close—once these format tweaks are applied, your build should pass CI cleanly. Nice work overall; tidy up the spacing and you’re done!


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

Footnotes

  1. Rate AI review example

package core.basesyntax;
public class Circle extends Figure {
private double radius;

Choose a reason for hiding this comment

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

This violates checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)". There's a redundant blank line immediately before the constructor. Please remove the empty line at line 4 so the constructor starts directly after the field declaration.

super(color);
this.radius = radius;
}

Choose a reason for hiding this comment

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

This violates checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)". There's a redundant blank line immediately before the getArea() method. Please remove the empty line at line 9 so the @Override/getArea() appear without a leading blank line.

public double getArea() {
return Math.PI * radius * radius;
}

Choose a reason for hiding this comment

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

This violates checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)". There's a redundant blank line immediately before the draw() method. Please remove the empty line at line 14 so the @Override/draw() appear without a leading blank line.

@@ -0,0 +1,14 @@
package core.basesyntax;
import java.util.Random;

Choose a reason for hiding this comment

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

This line is an empty (redundant) line right before the class declaration. This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove this blank line so the class declaration immediately follows the imports.

private Random random = new Random();
private static final String[] COLORS = {"red", "green", "blue", "yellow",
"orange", "magenta", "cyan", "white"};

Choose a reason for hiding this comment

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

This line is an empty (redundant) line between the fields and the method. This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove this blank line so the method declaration follows the fields without an extra leading empty line.

this.firstLeg = firstLeg;
this.secondLeg = secondLeg;
}

Choose a reason for hiding this comment

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

This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)". Remove the redundant blank line before the getArea() method so the @Override/getArea() start immediately after the constructor.

public double getArea() {
return (firstLeg * secondLeg) / 2.0;
}

Choose a reason for hiding this comment

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

This violates the checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)". Remove the redundant blank line before the draw() method so @Override/draw() begin without a leading empty line.

package core.basesyntax;
public class Square extends Figure {
private double side;

Choose a reason for hiding this comment

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

This violates the checklist item: "Don't begin class or method implementation with an empty line.\nRemove all redundant empty lines, be careful :)". Remove the redundant blank line so the constructor starts immediately after the field declaration.

super(color);
this.side = side;
}

Choose a reason for hiding this comment

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

This violates the checklist item: "Don't begin class or method implementation with an empty line.\nRemove all redundant empty lines, be careful :)". Remove the redundant blank line so the @OverRide and getArea() start without a leading empty line.

public double getArea() {
return side * side;
}

Choose a reason for hiding this comment

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

This violates the checklist item: "Don't begin class or method implementation with an empty line.\nRemove all redundant empty lines, be careful :)". Remove the redundant blank line so the @OverRide and draw() start without a leading empty line.

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