-
Notifications
You must be signed in to change notification settings - Fork 1.6k
add task solution #1979
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?
add task solution #1979
Conversation
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 several critical blockers that violate the required API/contract and may break functionality at runtime.
Critical issues to fix
- FigureSupplier API and logic:
- getDefaultFigure() name is misspelled as getDefautFigure() in both FigureSupplier and Calculator (Calculator line ~13). The task requires public Figure getDefaultFigure() that returns a white circle with radius 10. Rename the method and update all usages.
- getRandomFigure(): uses random.nextInt(4) while the switch has cases 0..4, making case 4 unreachable. Also, the default branch returns null, which can cause a NullPointerException when calling draw(). Define a constant for figure count (e.g., FIGURE_COUNT = 5), use random.nextInt(FIGURE_COUNT), and ensure every branch returns a valid Figure (or make default return getDefaultFigure()).
- Don’t pass expressions directly to constructors; create local variables for random values first to satisfy the checklist readability rule.
- ColorSupplier API mismatch: The method is declared public Color getRandomColor(), but the requirement is public String getRandomColor(). Change the signature to return String and convert enums using name() (e.g., Color.values()[i].name()). Ensure it never returns null.
- Enum string printing: In Circle, Rectangle, RightTriangle, Square, and IsoscelesTrapezoid draw() methods, you print the enum directly. The checklist requires using name(): replace getColor() with getColor().name() (or .name().toLowerCase() if lowercase is needed).
- Magic numbers: Replace hard-coded numbers with constants. Examples: new Figure[6] in Calculator (use FIGURE_ARRAY_SIZE), and random bounds/figure count in FigureSupplier.
Minor but required style fixes
- Remove redundant empty lines immediately after package declarations and between members across multiple files (AreaCalculable, Calculator, Circle, Color, ColorSupplier, Drawable, Figure, IsoscelesTrapezoid, Rectangle, RightTriangle, Square). This is a checklist requirement.
- Rename Rectangle constructor parameter randomWight to randomWidth for clarity.
Once you fix the API names, prevent null returns, adjust ColorSupplier to return String (or consistently handle enum.name() everywhere), replace magic numbers with constants, and clean up formatting, the solution should meet the task requirements. Good job on the overall structure and area/draw logic—after these corrections, you’ll be in great shape.
✨ 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,5 @@ | |||
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 :)" — remove the empty line between the package declaration and the interface declaration so the interface starts immediately after the package statement.
@@ -0,0 +1,20 @@ | |||
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.
There is a redundant empty line after the package declaration. The checklist requires: "Don't begin class or method implementation with an empty line." Remove this blank line to comply with the style rules.
public class Calculator { | ||
public static void main(String[] args) { | ||
FigureSupplier figureSupplier = new FigureSupplier(); | ||
Figure[] figures = new Figure[6]; |
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 array size is hard-coded as 6. The checklist requires that magic numbers be constants (UPPER_SNAKE_CASE). Define and use a constant (e.g., FIGURE_ARRAY_SIZE) instead of the literal 6.
} | ||
|
||
for (int i = figures.length / 2; i < figures.length; i++) { | ||
figures[i] = figureSupplier.getDefautFigure(); |
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.
You call getDefautFigure() but the required API name from the task is getDefaultFigure(). Fix the spelling here and ensure FigureSupplier implements public Figure getDefaultFigure() that returns a white circle with radius 10 as described in the task description.
@@ -0,0 +1,23 @@ | |||
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 redundant blank lines (e.g., line 2) so the class does not start with an empty line.
public Rectangle(Color randomColor, double randomWight, double randomHeight) { | ||
super(randomColor); | ||
this.width = randomWight; |
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.
Constructor parameter is named randomWight
(typo). Consider renaming it to randomWidth
to match the field width
and improve code readability. This is a non-critical suggestion (style/clarity) but worth fixing.
@@ -0,0 +1,26 @@ | |||
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 :)" Remove the blank line immediately after the package declaration so the class declaration follows the package statement without an empty line.
public void draw() { | ||
System.out.println("Figure: right triangle, area: " + getArea() | ||
+ " sq. units, legA: " + legA | ||
+ " units, legB: " + legB | ||
+ " units, color: " + getColor()); |
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: "Use name() for getting String representation of enum constants". The draw() method prints the color by concatenating getColor()
(lines 21–24), which depends on the enum's toString(). Use getColor().name()
(or getColor().name().toLowerCase()
if lowercase is required) to produce a predictable String representation of the color.
@@ -0,0 +1,23 @@ | |||
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. 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 so the class declaration follows the package immediately.
public void draw() { | ||
System.out.println("Figure: square, area: " + getArea() | ||
+ " sq. units, side: " + side | ||
+ " units, color: " + getColor()); |
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 prints the color by concatenating getColor()
(the enum instance). The checklist requires: "Use name() for getting String representation of enum constants". Use getColor().name()
(or getColor().name().toLowerCase()
if lowercase is required) when printing the color to avoid relying on toString()
behavior.
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 several critical blockers that violate the task spec and checklist, and they may break functionality or automated checks.
Critical issues to fix:
- FigureSupplier.getRandomFigure():
- Off-by-one bug using
random.nextInt(4)
while handling cases 0..4 (line ~10). Use a named constant (e.g.,FIGURE_COUNT = 5
) andrandom.nextInt(FIGURE_COUNT)
. - Default branch returns
null
(lines ~35-36), which violates “Don’t return null” and can cause NPEs whendraw()
is called. Ensure this method always returns a valid Figure.
- Off-by-one bug using
- API mismatch and typo: the method for the default figure is named
getDefautFigure()
(line ~41). Rename it togetDefaultFigure()
exactly and return a white circle with radius 10 (use aDEFAULT_RADIUS
constant). - ColorSupplier API mismatch:
public Color getRandomColor()
should bepublic String getRandomColor()
per the spec, and it should return an enum name vianame()
(optionally lowercase). Also introduce a constant for color count. - Calculator: uses a magic number for array size (
6
) and calls the misspelledgetDefautFigure()
. Replace the literal with a constant and callgetDefaultFigure()
. Also, relying on the current supplier may lead to NPEs when iterating and callingdraw()
.
Important quality fixes (after the blockers):
- Replace magic numbers throughout (array size, figure count, dim bounds, default radius) with named constants.
- In FigureSupplier, don’t pass random expressions directly to constructors; assign to local variables first.
- Ensure color printing uses explicit enum-to-string conversion:
getColor().name()
(consider.toLowerCase()
) across all draw() methods. - RightTriangle: rename
legA
/legB
tofirstLeg
/secondLeg
to match the required output labels. - Remove leading empty lines before class/interface declarations (AreaCalculable, Figure, Circle, Square) per the checklist.
You’ve got a solid structure and correct area formulas; once you address the API/logic issues and formatting rules above, the solution should pass checks. Keep going—these are quick, focused fixes that will bring everything into 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
@@ -0,0 +1,5 @@ | |||
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 the redundant empty line before the public interface AreaCalculable
declaration (line 2).
public class Calculator { | ||
public static void main(String[] args) { | ||
FigureSupplier figureSupplier = new FigureSupplier(); | ||
Figure[] figures = new Figure[6]; |
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 — magic number used for array size.
You declare the array with a literal 6
: Figure[] figures = new Figure[6];
. The checklist requires: "All magic numbers in your code should be constants." Consider introducing a named constant, for example:
private static final int ARRAY_SIZE = 6;
- then use
new Figure[ARRAY_SIZE]
and compute halves withARRAY_SIZE / 2
. This makes it clear and configurable and satisfies the checklist.
Figure[] figures = new Figure[6]; | ||
|
||
for (int i = 0; i < figures.length / 2; i++) { | ||
figures[i] = figureSupplier.getRandomFigure(); |
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.
Potential NullPointerException risk when assigning random figures.
You call figureSupplier.getRandomFigure()
and assign it directly: figures[i] = figureSupplier.getRandomFigure();
. The checklist requires: "Don't return null
from a method." Currently the FigureSupplier implementation contains a default
branch that returns null
, so figures[i]
might be null and cause problems later when draw()
is invoked. Either ensure FigureSupplier.getRandomFigure()
never returns null (fix its switch/default) or add a null-check/handling here. See FigureSupplier implementation for the null-returning branch.
} | ||
|
||
for (int i = figures.length / 2; i < figures.length; i++) { | ||
figures[i] = figureSupplier.getDefautFigure(); |
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.
API / spec mismatch: wrong method name for default figure.
You call figureSupplier.getDefautFigure();
(typo). The task description and checklist explicitly require a method named getDefaultFigure()
that returns a white circle with radius 10: "public Figure getDefaultFigure() - this method should always return a white circle with a radius of 10." Please rename/correct the method in FigureSupplier and call the exact name here. Leaving the misspelling will fail automated checks.
} | ||
|
||
for (Figure figure : figures) { | ||
figure.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.
Calling draw() without guarding against null figures.
You iterate for (Figure figure : figures) { figure.draw(); }
— if any element is null (see earlier comment about getRandomFigure possibly returning null), this will throw a NullPointerException at runtime. Either guarantee suppliers never return null (preferred per checklist) or check if (figure != null)
before calling draw()
. Checklist: "Don't return null
from a method."
private final double legA; | ||
private final double legB; |
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 requirement to use the specified property names for a right triangle. The task/description expects firstLeg
and secondLeg
for a right triangle (so output labels match the examples). Rename fields legA
-> firstLeg
and legB
-> secondLeg
, update constructor parameters and draw() labels accordingly. See: "firstLeg and secondLeg for right triangle".
public void draw() { | ||
System.out.println("Figure: right triangle, area: " + getArea() | ||
+ " sq. units, legA: " + legA | ||
+ " units, legB: " + legB | ||
+ " units, color: " + getColor()); |
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 prints the color by concatenating getColor()
(an enum). The checklist requires: "Use name() for getting String representation of enum constants". Change the print to use getColor().name().toLowerCase()
(or otherwise ensure the printed color is an English string) so output matches expected examples and checklist guidance.
@@ -0,0 +1,23 @@ | |||
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 requirement: "Don't begin class or method implementation with an empty line." Remove the empty line between the package declaration and the public class Square
declaration.
public void draw() { | ||
System.out.println("Figure: square, area: " + getArea() | ||
+ " sq. units, side: " + side | ||
+ " units, color: " + getColor()); |
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 guidance: "Use name() for getting String representation of enum constants". In draw()
you print getColor()
directly. Use getColor().name()
(and consider .toLowerCase()
to match example output like "blue") so the printed color is a proper String, e.g., ... + " color: " + getColor().name().toLowerCase()
.
|
||
private final double side; | ||
|
||
public Square(Color randomColor, double randomSide) { |
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 task description requires: "ColorSupplier with public String getRandomColor() method - for generating random color". Your constructor accepts a Color
enum (public Square(Color randomColor, ...)
). If you change ColorSupplier to return a String (as required), update constructors/Figure to accept/store a String color or convert consistently between String and enum. Ensure color types are consistent across the codebase.
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.
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 a few checklist/spec violations that are likely to fail automated checks and block acceptance, despite the overall good structure and correct area formulas.
Critical issues to fix
- RightTriangle naming mismatch (file: src/main/java/core/basesyntax/RightTriangle.java): The spec requires
firstLeg
andsecondLeg
. You currently uselegA
/legB
and print those labels. This breaks the requirement and may fail tests that check exact names. Rename fields, constructor parameters, and draw() labels tofirstLeg
andsecondLeg
. - Magic number in Calculator (file: src/main/java/core/basesyntax/Calculator.java line with
new Figure[6]
): The checklist requires constants for magic numbers. Add a named constant likeprivate static final int FIGURE_ARRAY_SIZE = 6;
and use it when creating the array. - Passing expressions directly to constructors (file: src/main/java/core/basesyntax/FigureSupplier.java multiple lines): The checklist says not to pass expressions directly in constructors. Assign random values to local variables first, then pass those variables into constructors (e.g.,
double radius = random.nextDouble(MIN_DIMENSION, MAX_DIMENSION);
).
Additional checklist/style fixes (minor but required)
- Use well-named constants in FigureSupplier: rename
MIN_INT
/MAX_INT
/DEFAULT_INT
toMIN_DIMENSION
/MAX_DIMENSION
/DEFAULT_RADIUS
, and ingetDefaultFigure()
use a constant for color (e.g.,DEFAULT_COLOR = Color.WHITE.name()
) and for radius. - Remove redundant leading/inner empty lines across files: AreaCalculable, Calculator, Circle, Color, ColorSupplier, Drawable, Figure, FigureSupplier, IsoscelesTrapezoid, Rectangle, RightTriangle, Square. The checklist forbids starting class/method bodies with empty lines. Remove the noted blank lines (e.g., AreaCalculable line 2; Calculator line 2; Circle lines 2,4,6,11,16; etc.).
Once you address the items above, the solution should fully comply with the task and pass checks. The overall design and formulas are solid—nice work; just tighten up these checklist items to finish strong.
✨ 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,5 @@ | |||
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 the blank line before the public interface AreaCalculable
declaration so the interface starts immediately after the package line.
@@ -0,0 +1,20 @@ | |||
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.
There is an empty line between the package declaration and the class declaration. 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 so the class starts immediately after the package declaration.
public class Calculator { | ||
public static void main(String[] args) { | ||
FigureSupplier figureSupplier = new FigureSupplier(); | ||
Figure[] figures = new Figure[6]; |
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 array size is declared with a literal 6
: Figure[] figures = new Figure[6];
. The checklist requires: "All magic numbers in your code should be constants." Replace this literal with a named constant like private static final int FIGURE_ARRAY_SIZE = 6;
and use it to create the array.
|
||
public class Circle extends Figure { | ||
|
||
private final double radius; | ||
|
||
public Circle(String randomColor, double randomRadius) { | ||
super(randomColor); | ||
this.radius = randomRadius; | ||
} | ||
|
||
@Override | ||
public double getArea() { | ||
return Math.PI * Math.pow(radius, 2); | ||
} | ||
|
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 at and after the top of the file and before class/method bodies (lines 2, 4, 6, 11, 16). This violates the checklist: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Please remove these empty lines so the class and its members start without leading blank lines.
@@ -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.
Remove the redundant empty line after the package declaration. The checklist says: "Don't begin class or method implementation with an empty line." Move the enum declaration up so it directly follows the package line.
public double getArea() { | ||
return (baseA + baseB) / 2 * height; | ||
} | ||
|
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 draw() method. Remove this blank line to comply with the checklist.
|
||
public class Rectangle extends Figure { | ||
|
||
private final double width; | ||
private final double height; | ||
|
||
public Rectangle(String randomColor, double randomWidth, double randomHeight) { | ||
super(randomColor); | ||
this.width = randomWidth; | ||
this.height = randomHeight; | ||
} | ||
|
||
@Override | ||
public double getArea() { | ||
return width * height; | ||
} | ||
|
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 at and after the top of the file and before class members (lines 2, 4, 7, 13, 18). This violates the checklist: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Please remove these empty lines so the class and its members start without leading blank lines. (See Rectangle.java: )
private final double legA; | ||
private final double legB; | ||
|
||
public RightTriangle(String randomColor, double randomLegA, double randomLegB) { | ||
super(randomColor); | ||
this.legA = randomLegA; | ||
this.legB = randomLegB; |
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 class uses legA
and legB
for its fields and prints legA
/legB
in draw(). This violates the spec requirement: "RightTriangle: firstLeg and secondLeg". Rename fields, constructor parameters and draw() labels to firstLeg
and secondLeg
so the API and printed output match the requirement exactly. See RightTriangle.java for current code and the task description for the requirement.
|
||
public class RightTriangle extends Figure { | ||
|
||
private final double legA; | ||
private final double legB; | ||
|
||
public RightTriangle(String randomColor, double randomLegA, double randomLegB) { | ||
super(randomColor); | ||
this.legA = randomLegA; | ||
this.legB = randomLegB; | ||
} | ||
|
||
@Override | ||
public double getArea() { | ||
return (legA * legB) / 2; | ||
} | ||
|
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 at and inside the file (lines 2, 4, 7, 13, 18). This violates the checklist: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)". Remove these blank lines so the class and its members start without leading empty lines.
|
||
public class Square extends Figure { | ||
|
||
private final double side; | ||
|
||
public Square(String randomColor, double randomSide) { | ||
super(randomColor); | ||
this.side = randomSide; | ||
} | ||
|
||
@Override | ||
public double getArea() { | ||
return Math.pow(side, 2); | ||
} | ||
|
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 at and after the top of the file and before class members (lines 2, 4, 6, 11, 16). This violates the checklist: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Please remove these empty lines so the class and its members start without leading blank lines.
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 to make your solution even better next time.
What you did well:
- All figure classes are implemented correctly, with proper fields, area calculations, and draw() output in English.
- Good separation of concerns: Figure base class plus separate interfaces (AreaCalculable, Drawable), and proper suppliers (ColorSupplier, FigureSupplier). The main flow in Calculator is correct and uses a single FigureSupplier instance.
What to improve next:
- FigureSupplier: generate random dimensions as local variables inside getRandomFigure(), not as class-level final fields. This ensures fresh values per call and follows the checklist: "Creating a figure, don't pass expressions in the constructor. Create separate variables and pass them on for better code readability." For example: double radius = random.nextDouble(MIN_DIMENSION, MAX_DIMENSION); then return new Circle(colorSupplier.getRandomColor(), radius).
- Remove redundant empty lines per the checklist: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Specific spots to fix include:
- AreaCalculable.java: remove blank line at line 2.
- Calculator.java: remove blank lines after package, after class declaration, after main signature, and between loops.
- Circle.java, Color.java, ColorSupplier.java, Drawable.java, Figure.java, FigureSupplier.java, IsoscelesTrapezoid.java, Rectangle.java, RightTriangle.java, Square.java: remove the blank line after the package and any blank lines immediately before fields, constructors, or methods.
Minor tip:
- Keep class members compact by avoiding extra blank lines between fields, constructors, and methods unless grouping is intentional.
Great job on functionality and structure—polishing randomness generation and formatting will make your solution fully checklist-compliant!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
@@ -0,0 +1,5 @@ | |||
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 starts the interface implementation with an empty line. 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 line 2 so the interface declaration immediately follows the package statement.
@@ -0,0 +1,24 @@ | |||
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 blank line immediately after the package statement. 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 to comply with the guideline.
package core.basesyntax; | ||
|
||
public class Calculator { | ||
|
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 blank line right after the class declaration before the first field. The checklist requires: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove this empty line so the first field appears directly after the class opening.
private static final int FIGURE_ARRAY_SIZE = 6; | ||
|
||
public static void main(String[] args) { | ||
|
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 main method signature. 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 so the method body starts without a leading empty line.
|
||
FigureSupplier figureSupplier = new FigureSupplier(); | ||
Figure[] figures = new Figure[FIGURE_ARRAY_SIZE]; | ||
|
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 inside the method before the first loop. The checklist requires removal of redundant empty lines: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove this blank line to make the method body more compact.
@@ -0,0 +1,23 @@ | |||
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.
Violates checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" — there is a blank line immediately after the package statement; remove it.
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.
Violates checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" — there is a blank line right after the class declaration before the first field; remove it.
public class Square extends Figure { | ||
|
||
private final double 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.
Violates checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" — there is a blank line between the field declaration and the constructor; remove it.
super(randomColor); | ||
this.side = randomSide; | ||
} | ||
|
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.
Violates checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" — there is a blank line between the constructor and the @OverRide annotation for getArea(); remove it.
public double getArea() { | ||
return Math.pow(side, 2); | ||
} | ||
|
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.
Violates checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" — there is a blank line between getArea() method and the @OverRide annotation for draw(); remove it.
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
The test task has already been approved.
[CODE: AAP]
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.
No description provided.