-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Version1/Created working program according to task #1977
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
base: master
Are you sure you want to change the base?
Conversation
…child classes and 2 support classes FigureSupplier and ColorSupplier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Summary
Requesting changes because several critical, blocking issues violate explicit task requirements and checklist rules.
Critical issues to fix
- draw() must print and return void: In Draw.java and all figures (Square, Rectangle, RightTriangle, Circle, IsoscelesTrapezoid),
draw()
currently returns String. Per requirement: “To ‘draw’ means to print out all information about a figure using System.out.println().” Update the interface tovoid draw()
and have each figure callSystem.out.println(...)
inside draw. In Main.java, replaceSystem.out.println(figure.draw());
withfigure.draw();
. - Separate responsibilities and expose area: There is no public
getArea()
(or Area interface). The checklist requires keepingdraw()
andgetArea()
separate. Addpublic float getArea()
(or implement a dedicated interface such asHasArea
) across figures and use it from draw(). - Incorrect area formula in IsoscelesTrapezoid: Current code does integer division in
(height / 2)
. Use floating-point division: for examplearea = (horizontalSide + verticalSide) * (float) height / 2f;
. Also fix the printed label to “Figure: isosceles trapezoid”. - Don’t return null: FigureSupplier.getRandomFigure() has
default -> null
. Replace with a non-null fallback (e.g.,getDefaultFigure()
), as returning null violates the checklist and risks NPEs. - Ensure positive random dimensions and avoid passing expressions directly: In FigureSupplier, use locals and guarantee > 0 (e.g.,
int side = random.nextInt(MAX_PARAMETER) + 1;
) before passing to constructors, rather than inline expressions.
Important design/style fixes (still required by checklist)
- Rename interface from the verb
Draw
to a noun likeDrawable
. - Remove redundant leading empty lines across files (e.g., Circle.java, Color.java, ColorSupplier.java, Draw.java, Figure.java, FigureSupplier.java, IsoscelesTrapezoid.java, Main.java, Rectangle.java, RightTriangle.java, Square.java).
- In ColorSupplier, reuse a
private final Random random = new Random();
instead ofnew Random()
per call. - Replace magic number
10
ingetDefaultFigure()
with a named constant (e.g.,DEFAULT_CIRCLE_RADIUS
). - Make draw() outputs consistent in units (linear: “units”; area: “sq. units”).
Why this is required
- These issues are critical because they break core functional requirements (printing inside draw, correct area calculation) and explicit checklist items (no null returns, naming, separation of concerns). They will lead to wrong output and likely failing checks.
You’ve set up the overall structure well (all figures, suppliers, and enum exist). After making the changes above, your solution should fully comply. Keep going—you’re close!
✨ 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
@@ -0,0 +1,18 @@ | |||
package core.basesyntax; | |||
|
There was a problem hiding this comment.
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 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 :)" Please remove the blank line so the class starts directly after the package declaration.
|
||
public class Circle extends Figure { | ||
private final int radius; | ||
|
There was a problem hiding this comment.
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 immediately before the constructor. The checklist requires: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove this redundant blank line so the constructor starts without a leading empty line.
this.color = color; | ||
area = (float) Math.PI * (radius * radius); | ||
} | ||
|
There was a problem hiding this comment.
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 between the constructor and the overridden draw method. The checklist says: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove this empty line so the method's @OverRide and signature start immediately after the constructor.
} | ||
|
||
@Override | ||
public String draw() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The draw() method currently returns a String. The task description requires: "To 'draw' means to print out all information about a figure using System.out.println() (you shouldn't override the toString() method for this)." Please change draw() to perform the printing inside the method (use System.out.println) and make it return void.
public Circle(int radius, String color) { | ||
this.radius = radius; | ||
this.color = color; | ||
area = (float) Math.PI * (radius * radius); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Area is calculated in the constructor and stored in the area
field, but the class does not provide a getArea()
method or implement an area-specific interface. The checklist advises separating area computation from drawing: "All our classes and interfaces should have a single purpose - the draw()
and getArea()
methods are not conceptually close to each other." Add a public getArea()
(or implement a dedicated Area interface) so other code can obtain the figure's area explicitly.
@@ -0,0 +1,18 @@ | |||
package core.basesyntax; | |||
|
There was a problem hiding this comment.
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 right 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 :)" Please remove the blank line so the class declaration follows the package line immediately.
|
||
public class Square extends Figure { | ||
private final int side; | ||
|
There was a problem hiding this comment.
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 between the field declarations and the constructor. The checklist requires: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove this redundant blank line so the constructor starts directly after the fields.
this.color = color; | ||
area = side * side; | ||
} | ||
|
There was a problem hiding this comment.
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 between the constructor and the overridden method. Remove this empty line to comply with the checklist: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)"
} | ||
|
||
@Override | ||
public String draw() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The draw() method currently returns a String (public String draw()
). The task description requires: "To 'draw' means to print out all information about a figure using System.out.println() (you shouldn't override the toString() method for this)." Change draw() to be void and perform the printing inside the method (use System.out.println).
public Square(int side, String color) { | ||
this.side = side; | ||
this.color = color; | ||
area = side * side; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Area is computed and stored in the area
field but the class does not expose a getArea() method or implement an Area-specific interface. The checklist recommends separating area computation from drawing: "Don't put all behavior into a single interface if the methods are conceptually different from each other. All our classes and interfaces should have a single purpose - the draw()
and getArea()
methods are not conceptually close to each other." Add a public getArea()
(or implement a dedicated Area interface) so callers can obtain the area explicitly and draw() can call that method when printing.
There was a problem hiding this 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 several core requirements are not met and they block task completion.
Critical issues to fix:
- Draw contract violation across the project: draw() must print via System.out.println(), not return a String. Update the interface to a noun like Drawable with
void draw()
, then change all figures (Circle, Square, Rectangle, RightTriangle, IsoscelesTrapezoid) to print inside draw(), and in Main callfigure.draw();
(e.g., Main line 18). - Missing public area access: the task requires obtaining the area of any figure. Add a public
getArea()
(preferably in Figure or via a dedicated Area interface) and return the protectedarea
(e.g., Figure and each concrete class). - IsoscelesTrapezoid: area formula is wrong due to integer division (line ~13). Use float arithmetic:
(topBase + bottomBase) * (float) height / 2f
. Also fix draw text to identify the correct figure and use proper units. - FigureSupplier.getRandomFigure(): do not return null in the default branch; return a valid figure (e.g.,
getDefaultFigure()
). Ensure random dimensions are positive non‑zero (use+1
or MIN/MAX). Don’t pass expressions directly to constructors—assign to local variables first. Extract the default radius 10 into a constant.
Minor but important improvements:
- ColorSupplier: move
new Random()
to a private final field; remove redundant blank lines. - Remove redundant empty lines at the top/inside multiple files (Draw, Figure, ColorSupplier, FigureSupplier, Main, and all figures) per the checklist.
- Fix unit labels in draw outputs: lengths use “units”, only area uses “sq. units”.
You’ve modeled shapes and most area formulas well—great start. Address the draw printing contract, expose getArea(), fix the trapezoid formula/labels, and clean up Supplier and formatting to meet the checklist and task requirements.
✨ 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
@Override | ||
public String draw() { | ||
return "Figure: circle, area: " + area | ||
+ " sq. units, radius: " + radius | ||
+ " units, color: " + color; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This violates the task description: "To 'draw' means to print out all information about a figure using System.out.println() (you shouldn't override the toString() method for this)." Currently Draw.draw() returns a String and Circle.draw() returns a String; Circle.draw() should print directly via System.out.println and return void. See Draw.java and the description.
Suggestion: change the Draw interface to void draw()
and implement System.out.println("Figure: circle, area: ...")
inside Circle.draw().
@@ -0,0 +1,18 @@ | |||
package core.basesyntax; | |||
|
|||
public class Circle extends Figure { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This violates the requirement that callers must be able to obtain a figure's area. The description states: "we can obtain the area of any figure and are able to draw it." You currently compute and store the area in the protected field but do not expose it via a public getArea()
method or an area-specific interface. Add public float getArea()
(or an AreaCalculator interface) so other code can retrieve the area. See Figure.java which holds the protected area field.
|
||
public class Circle extends Figure { | ||
private final int radius; | ||
|
||
public Circle(int radius, String color) { | ||
this.radius = radius; | ||
this.color = color; | ||
area = (float) Math.PI * (radius * radius); | ||
} | ||
|
There was a problem hiding this comment.
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(s) before the class declaration and the extra blank lines inside the class (lines 2, 5 and 11). Cleaner files should not start or separate members with unnecessary empty lines.
@@ -0,0 +1,9 @@ | |||
package core.basesyntax; | |||
|
There was a problem hiding this comment.
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 are redundant empty lines at the top of the file (line 2). Remove unnecessary blank lines to comply with the style guidance.
|
||
public class ColorSupplier { | ||
public String getRandomColor() { | ||
return Color.values()[new Random().nextInt(Color.values().length)].name(); |
There was a problem hiding this comment.
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: "Think about which variables should be local in the method and which should be class-level". A new Random() is created on every method call (line 7). Instead, declare a private final Random random = new Random(); at class level and reuse it in getRandomColor() to avoid repeated allocations and follow the checklist guidance.
@@ -0,0 +1,20 @@ | |||
package core.basesyntax; | |||
|
|||
public class RightTriangle extends Figure { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This violates the requirement: "All figures must be able to compute their area via a getArea() behavior accessible for any figure instance." The class computes area
but does not expose a public getArea()
method. Add public float getArea()
(or implement an AreaCalculator interface) so callers can obtain the area. Refer to the abstract Figure or an area-specific interface to place this method appropriately.
return "Figure: right triangle, area: " + area + " sq. units, first leg: " | ||
+ firstLeg + " sq. units, second leg: " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This violates the requirement to print dimensions with correct units: the draw output uses "sq. units" for firstLeg
(line 16). Linear dimensions should be labeled with "units" while area must use "sq. units". Update the output to something like: ..., area: <area> sq. units, firstLeg: <value> units, secondLeg: <value> units, color: <color>
.
|
||
public class Square extends Figure { | ||
private final int side; | ||
|
||
public Square(int side, String color) { | ||
this.side = side; | ||
this.color = color; | ||
area = side * side; | ||
} | ||
|
There was a problem hiding this comment.
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 lines at the top and inside the class (for example lines 2, 5 and 11) to follow the style rule. See file contents.
@Override | ||
public String draw() { | ||
return "Figure: square, area: " + area | ||
+ " sq. units, side: " + side | ||
+ " units, color: " + color; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This violates the requirement from the task description: "To 'draw' means to print out all information about a figure using System.out.println() (you shouldn't override the toString() method for this)." Currently draw()
is declared to return a String
(Draw interface) and Square.draw()
returns a String
instead of printing. Change the Draw interface to void draw()
and implement System.out.println(...)
inside Square.draw()
(then call square.draw()
from Main). See Draw.java and Main.java for current usage.
@@ -0,0 +1,18 @@ | |||
package core.basesyntax; | |||
|
|||
public class Square extends Figure { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This violates the requirement: the description states we must be able to obtain a figure's area: "we can obtain the area of any figure and are able to draw it." You currently compute and store area
in the protected field (Figure) but there is no public getArea()
(or AreaCalculator interface). Add public float getArea()
(or an AreaCalculator interface) so callers can access the area. See Figure.java which holds the protected area
.
…mplement getArea method, fix trapezoid area formula, fix checkstyle mistakes according to AIBuddy.
There was a problem hiding this 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 violate the task requirements and checklist and can break expected behavior/output.
Critical issues to fix:
- Public area API is missing across figures:
getArea()
is declaredprotected
in Figure and overridden asprotected
in concrete classes (e.g., Circle line 19; RightTriangle line 21; Rectangle; Square; IsoscelesTrapezoid). The task requires that “we can obtain the area of any figure,” so callers must be able to invokegetArea()
publicly. Suggested fix: introduce a dedicated public interface (e.g.,public interface Area { float getArea(); }
) and implement it in each figure withpublic float getArea()
. Also remove the abstract behavior fromFigure
to follow the checklist (“Don’t use abstract classes to set behavior for classes”). - IsoscelesTrapezoid.draw() prints the wrong figure type and has formatting issues (file IsoscelesTrapezoid.java): it currently prints “Figure: rectangle, …” and misses units/comma before color. This breaks the required output. Fix the label to “Figure: isosceles trapezoid” and ensure consistent formatting: include units for height and proper separators.
- FigureSupplier.getRandomFigure() passes expressions directly into constructors and allows zero-sized dimensions (lines 16–26). This violates the checklist and can generate invalid figures (dimension zero). Compute local variables first and ensure positive ranges, e.g.,
int side = random.nextInt(MAX_PARAMETER) + 1; String color = colorSupplier.getRandomColor(); return new Square(side, color);
. Optionally extract a helper likeprivate int nextParam()
to remove inline+ 1
magic.
Minor but required by checklist (style): remove redundant empty lines immediately after package declarations in multiple files (Circle.java line 2, Color.java line 2, ColorSupplier.java lines 2 and 4 and before method, Drawable.java line 2, Figure.java line 2, FigureSupplier.java line 2, IsoscelesTrapezoid.java line 2, Main.java line 2, Rectangle.java line 2, RightTriangle.java line 2, Square.java line 2).
Once you make getArea()
public via an interface, correct the trapezoid’s draw output, and fix FigureSupplier’s parameter generation, your solution will meet the core requirements. Good job on having all five figures, proper use of Drawable and ColorSupplier, and correct formulas in most classes—these fixes will bring everything in line with the checklist and task specs.
✨ 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
@@ -0,0 +1,22 @@ | |||
package core.basesyntax; | |||
|
|||
public class Circle extends Figure { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class extends the abstract Figure
, which is being used to provide behavior. The checklist explicitly says: "Don't use abstract classes to set behavior for classes". Prefer moving behavior into interfaces (e.g., an Area
interface for area calculation) and have concrete figures implement those interfaces instead of relying on an abstract class for behavior.
} | ||
|
||
@Override | ||
protected float getArea() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getArea() is declared protected
here, which prevents external callers from obtaining the area. The task requires that "we can obtain the area of any figure" and the checklist advises separating draw()
and getArea()
responsibilities: "All our classes and interfaces should have a single purpose - the draw()
and getArea()
methods are not conceptually close to each other." Make getArea()
public (e.g., by introducing a public Area
interface with public float getArea()
and implementing it in Circle).
@@ -0,0 +1,22 @@ | |||
package core.basesyntax; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the redundant empty line after the package declaration. The checklist notes: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)"
@@ -0,0 +1,10 @@ | |||
package core.basesyntax; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file begins with an empty line after the package declaration which violates the checklist: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove the blank line so the enum immediately follows the package statement.
@@ -0,0 +1,11 @@ | |||
package core.basesyntax; | |||
|
There was a problem hiding this comment.
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 :)" Remove the blank line after the package declaration (line 2).
@@ -0,0 +1,24 @@ | |||
package core.basesyntax; | |||
|
|||
public class RightTriangle extends Figure { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class extends the abstract Figure
which currently carries behavior (getArea). The checklist says: "Don't use abstract classes to set behavior for classes" — move behavioral contracts (area calculation) into a focused interface (e.g., Area
) and have concrete figures implement that interface instead of relying on an abstract class for behavior.
} | ||
|
||
@Override | ||
protected float getArea() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getArea() is declared protected float getArea()
which prevents external callers from obtaining the area. The task description requires that "we can obtain the area of any figure and are able to draw it." Make getArea()
part of a public interface (for example Area
) and implement it as public float getArea()
in RightTriangle so callers can access the area.
@@ -0,0 +1,22 @@ | |||
package core.basesyntax; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Violation: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" There is a redundant empty line after the package declaration. Remove the blank line so the class starts immediately after the package statement.
@@ -0,0 +1,22 @@ | |||
package core.basesyntax; | |||
|
|||
public class Square extends Figure { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Violation: "Don't use abstract classes to set behavior for classes" — Square extends Figure
which is being used to provide behavior (getArea). The checklist expects behavior to be provided via interfaces rather than abstract classes. Consider moving area behavior into a dedicated interface (e.g., Area
) and have Square implement it instead of relying on an abstract superclass.
} | ||
|
||
@Override | ||
protected float getArea() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requirement & checklist mismatch: The description requires that "we can obtain the area of any figure and are able to draw it." Currently getArea()
is declared protected
("protected float getArea()") which prevents external callers from obtaining the area. Make getArea()
public (for example by introducing a public Area
interface with public float getArea()
and implementing it in Square). This also satisfies: "Don't put all behavior into a single interface if the methods are conceptually different from each other."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
…s, implement zero-sized dimension in FigureSupplier, fix trapezoid wrong text writing.
There was a problem hiding this 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: moving forward while noting a few improvements. Overall, you implemented the figures set well—area formulas are correct, draw() outputs are structured and in English, suppliers are reused correctly in Main, and ColorSupplier uses enum.name() as required.
Critical fixes to address next:
- IsoscelesTrapezoid.draw(): change the printed figure label from "Figure: rectangle" to "Figure: isosceles trapezoid" so it matches the required format and the actual class.
- Design per checklist: move
getArea()
out of Figure into a dedicated interface (e.g.,Area
withfloat getArea()
), then have each concrete figure implement it; remove the defaultgetArea()
from Figure. This aligns with “prefer interfaces for behavior” and separating unrelated behaviors.
Style (redundant empty lines) to clean up for checklist compliance:
- Remove leading blank lines after package and before class/method bodies in: Circle (lines 2, 5, 10, 17), Color (line 2), ColorSupplier (lines 2, 4, 7), Drawable (line 2), Figure (lines 2, 5, 9, 12), FigureSupplier (lines 2, 4, 7), IsoscelesTrapezoid (top of file; between ctor and @OverRide; between draw and getArea), Main (lines 2, 6, 9, 16), Rectangle (lines 2, 6, 12, 19), RightTriangle (top; between fields/ctor; between ctor/draw; between draw/getArea), Square (top; between field/ctor; before @OverRide draw; before @OverRide getArea).
What you did well: correct formulas, clear output structure for most figures, proper constants and suppliers usage, and good separation of randomness into suppliers. With the quick label fix and formatting cleanup, plus the small interface refactor, this will fully match the checklist. Keep up the solid progress!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
@@ -0,0 +1,22 @@ | |||
package core.basesyntax; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file begins with an empty line right after the package declaration. The checklist requires: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Please remove this redundant blank line (delete line 2).
|
||
public class Circle extends Figure { | ||
private final int radius; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an unnecessary empty line before the constructor. The checklist requires: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Please remove the empty line at line 5.
|
||
@Override | ||
public void draw() { | ||
System.out.println("Figure: circle, area: " + getArea() | ||
+ " sq. units, radius: " + radius | ||
+ " units, color: " + color); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are redundant empty lines before/around the overrides and methods (lines 10 and 17). The checklist requires: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove these blank lines so annotations and methods start immediately without a leading empty line.
@@ -0,0 +1,10 @@ | |||
package core.basesyntax; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file begins with a redundant empty line 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 :)" Please remove the empty line at line 2 so the enum starts immediately after the package statement.
@@ -0,0 +1,11 @@ | |||
package core.basesyntax; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file has a redundant empty line immediately after the package declaration. The checklist explicitly says: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Please remove this blank line so the import follows the package declaration directly.
+ firstLeg + " units, second leg: " | ||
+ secondLeg + " units, color: " + color); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a redundant blank line between the draw() method and the getArea() method. The checklist says: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Please remove this blank line to comply.
@@ -0,0 +1,22 @@ | |||
package core.basesyntax; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file has a redundant empty line after the package declaration. The checklist requires: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove the blank line so the import/class follows immediately.
|
||
public class Square extends Figure { | ||
private final int side; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an unnecessary blank line between the field declaration and the constructor. The checklist requires: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove this blank line.
this.side = side; | ||
this.color = color; | ||
} | ||
|
There was a problem hiding this comment.
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 immediately after the constructor closing brace and before the @Override
draw annotation. The checklist requires: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove this blank line.
+ " sq. units, side: " + side | ||
+ " units, color: " + color); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an extra blank line between the end of draw() and the @Override
getArea() annotation. The checklist requires: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove this blank line.
There was a problem hiding this 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: moving forward so you can keep learning. Overall, your solution meets the core functional requirements: figures compute areas correctly, draw() prints details in English, suppliers generate valid data, and Main builds and prints the array as required. Nice job separating drawing via the Drawable interface and using Color.name() for output.
Key fixes to address next:
- Output correctness: In src/main/java/core/basesyntax/IsoscelesTrapezoid.java, draw() prints the wrong figure label. Change the heading to “Figure: isosceles trapezoid” so it matches the actual type and the required format (area in “sq. units”, each parameter with “units”, then color). Example: System.out.println("Figure: isosceles trapezoid, area: " + getArea() + " sq. units, ...").
- Structural requirement (checklist 3.1): Add a dedicated area interface, e.g., public interface Area { float getArea(); }, and implement it in your figures (or in Figure). This separates concerns so draw() and getArea() aren’t bundled in one place.
- Style (checklist: “Don’t begin class or method implementation with an empty line”): Remove redundant empty lines immediately after package declarations and before constructors/methods in these files: Circle.java, Color.java, ColorSupplier.java, Drawable.java, Figure.java, FigureSupplier.java, IsoscelesTrapezoid.java, Main.java, Rectangle.java, RightTriangle.java, Square.java. This typically means deleting the blank line at the top and any blank line right before a constructor/@OverRide method.
You’re very close—functionality is solid. Clean up the formatting, fix the trapezoid label, and introduce the Area interface to fully align with the checklist. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
@@ -0,0 +1,22 @@ | |||
package core.basesyntax; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant empty line after package declaration. The checklist requires not to begin class/method implementation with an empty line—remove this blank line so the class declaration follows the package statement directly.
|
||
public class Circle extends Figure { | ||
private final int radius; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant empty line before the constructor. Remove this empty line so the constructor starts immediately after the field declarations (checklist: don't begin method implementation with an empty line).
this.radius = radius; | ||
this.color = color; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant empty line before the @OverRide of draw(). Remove the blank line so the annotation and method start directly after the previous code block (style checklist).
+ " sq. units, radius: " + radius | ||
+ " units, color: " + color); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant empty line before the @OverRide of getArea(). Remove the blank line so the annotation and method start directly after the previous code block (style checklist).
@@ -0,0 +1,10 @@ | |||
package core.basesyntax; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file contains a redundant empty line after the package declaration. Per 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 file starts directly with the enum declaration.
+ firstLeg + " units, second leg: " | ||
+ secondLeg + " units, color: " + color); | ||
} | ||
|
There was a problem hiding this comment.
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 before the @OverRide of getArea() so the annotation and method start directly after the preceding code block.
@@ -0,0 +1,22 @@ | |||
package core.basesyntax; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant empty line after package declaration. The checklist requires not to begin class/method implementation with an empty line — remove this blank line so the class declaration follows the package statement directly.
|
||
public class Square extends Figure { | ||
private final int side; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant empty line before the constructor. Remove this blank line so the constructor starts immediately after the field declarations (checklist rule).
this.side = side; | ||
this.color = color; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant empty line before the @OverRide of draw(). Remove the blank line so the annotation and method start directly after the previous code block (style checklist).
+ " sq. units, side: " + side | ||
+ " units, color: " + color); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant empty line before the @OverRide of getArea(). Remove the blank line so the annotation and method start directly after the previous code block (style checklist).
Created parent class Figure, interface class Draw, 5 Figure child classes and 2 support classes FigureSupplier and ColorSupplier