-
Notifications
You must be signed in to change notification settings - Fork 1.6k
full implementation of OOP Advanced task #1976
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
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
Something went wrong while processing the review.
[CODE: GAE]
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
Decision: REQUEST_CHANGES — there are several critical checklist/spec violations that block acceptance.
Critical issues to fix
- Random dimensions and constructor arguments (FigureSupplier.java): You pass inline
rg.nextDouble()
directly to constructors, producing values in [0,1) (including 0). This violates the checklist (“Don’t pass expressions into constructors” and “All magic numbers should be constants”) and can yield zero/invalid dimensions. How to fix: add constants in Constant.java (e.g.,MIN_DIMENSION
,MAX_DIMENSION
) and compute locals before construction, for example:double radius = MIN_DIMENSION + rg.nextDouble() * (MAX_DIMENSION - MIN_DIMENSION);
thennew Circle(color, radius)
. - Missing constants (Constant.java): Add
MIN_DIMENSION
,MAX_DIMENSION
, andDEFAULT_COLOR
. Use them from FigureSupplier for random generation and default figure color. This is required by “All magic numbers should be constants.” Also align default color with the spec’s “white” (decide on case; if lowercase is required, useDEFAULT_COLOR = "white"
). - Square API mismatch (Square.java): Square must expose a single property
side
. Currently it inherits two dimensions (bottom
,heigh
) and ignores one in area. Refactor Square to have a singleside
field and computeside * side
. Update draw() to printside
and adjust FigureSupplier to pass one side variable. - RightTriangle required properties (RightTriangle.java): The spec requires
firstLeg
andsecondLeg
. Refactor fields/constructor accordingly and updategetArea()
to(firstLeg * secondLeg) / Constant.DIVISION_FOR_AREA
. Update draw() labels to match. - Systematic misspelling
heigh
→height
(FigureWithBottomAndHeigh.java, IsoscelesTrapezoid.java, Rectangle.java, RightTriangle.java, Square.java): This violates “Use only eng in messages/code.” Rename the base class (e.g.,FigureWithBottomAndHeight
or better, rename fields per-figure), fix field/parameter names, and update all usages and printed labels.
Minor but required style fixes (easy wins)
- Remove leading empty lines after package declarations in: Application.java, AreaCalculator.java, Drawer.java, FigureWithBottomAndHeigh.java, RightTriangle.java, Square.java, IsoscelesTrapezoid.java. Checklist: “Don’t begin class or method implementation with an empty line.”
- Improve draw() formatting consistency (commas and labels). Example:
Figure: rectangle, area: <area> sq. units, width: <w> units, height: <h> units, color: <COLOR>
.
What’s good
- Core architecture is solid: Figure base class with Drawer and AreaCalculator interfaces, working ColorSupplier, and correct Application flow (single FigureSupplier instance; first half random, second half default; then draw()). Circle implementation is correct.
Please address the above blockers (constants, random generation, Square/RightTriangle property refactors, and the height misspelling) and the small style fixes. After these changes, your solution should meet the checklist and pass expected validations. 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,15 @@ | |||
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 the class implementation with an empty line. The checklist states: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)". Please remove the blank line between the package declaration and the class declaration so the class starts immediately after the package statement .
@@ -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 has a redundant empty line immediately after the package declaration. This violates the checklist instruction: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Consider removing the blank line so the interface declaration directly follows the package statement.
|
||
public class Constant { | ||
public static final int NUMBER_OF_COLORS = Colors.values().length; | ||
public static final int NUMBER_OF_FIGURES = 5; |
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 is missing constants for minimum/maximum bounds used when generating random dimensions. The checklist requires that "All magic numbers in your code should be constants." Add constants such as MIN_DIMENSION and MAX_DIMENSION (double) and use them from FigureSupplier instead of calling Random.nextDouble() without scaling. See checklist: "All magic numbers in your code should be constants."
public static final int NUMBER_OF_COLORS = Colors.values().length; | ||
public static final int NUMBER_OF_FIGURES = 5; | ||
public static final int DIVISION_FOR_AREA = 2; | ||
public static final int DEFAULT_CIRCLE_RADIUS = 10; |
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 have DEFAULT_CIRCLE_RADIUS (10) which is required, but the project also needs a constant for the default color. The task states: "public Figure getDefaultFigure() - this method should always return a white circle with a radius of 10." Add a constant like DEFAULT_COLOR = "white" (or define and document using Colors.WHITE.name()) so the default figure color is driven by a constant rather than a literal elsewhere. Quote from description: "public Figure getDefaultFigure() - this method should always return a white circle with a radius of 10."
@@ -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 contains a redundant empty line 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 blank line at line 2 so the interface declaration immediately follows the package declaration.
public void draw() { | ||
System.out.println("Figure: right triangle, area: " + area + " sq. units, bottom: " | ||
+ bottom + " units, heigh: " + heigh + " 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.
The draw() output prints the misspelled identifier heigh
and labels like bottom
. The checklist asks to "Use only eng in messages/code:" and the task expects the triangle to show its legs (e.g., firstLeg
and secondLeg
). Update the printed message to use correct names and spelling after refactoring, for example: "Figure: right triangle, area: sq. units, firstLeg: units, secondLeg: units, color: ". Also, ensure consistent punctuation/commas across all draw() outputs .
@@ -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.
This file begins with an empty line after the package declaration. The checklist explicitly states: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Please remove the redundant blank line at the top of the file to comply with the checklist 【.
|
||
public class Square extends FigureWithBottomAndHeigh { | ||
|
||
public Square(String color, double bottom, double heigh) { |
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.
Square currently extends FigureWithBottomAndHeigh and its constructor accepts bottom
and heigh
. According to the task description each figure must expose its own specific property(ies) (for example: radius
for circle, firstLeg
and secondLeg
for right triangle). A square should have a single property side
. This class violates that requirement by modelling two dimensions and keeping heigh
unnecessarily. Consider refactoring Square to accept and store a single side
(and compute area as side * side
) so the API reflects the figure-specific property required by the description 【.
public double getArea() { | ||
return bottom * bottom; |
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() uses bottom * bottom
and ignores the inherited heigh
value. That demonstrates the API mismatch: Square should declare and use side
rather than relying on bottom
/heigh
from the parent. Update the class to have a side
field and return side * side
here. This will also make the constructor and state consistent and avoid storing an unused heigh
value.
public void draw() { | ||
System.out.println("Figure: square, area: " + area + " sq. units, bottom: " | ||
+ bottom + " units, heigh: " + heigh + " 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.
draw() prints bottom
and the misspelled heigh
rather than side
, and the output lacks consistent punctuation. The task requires draw() to print figure type, area, specific dimensions and color in a clear format (see examples in the task description). Also fix the misspelling heigh
→ height
across the codebase. Example desired output: "Figure: square, area: 25.0 sq. units, side: 5.0 units, color: BLUE". Update the printed labels and spelling accordingly to match the expected output style and the requirement to use English in messages/code 【.
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 critical blockers that can break the build or runtime behavior.
Critical issues to fix:
- FigureSupplier.java: using
rg.nextDouble(bound)
will not compile (standard Random has no such overload). Replace with the scaled pattern, e.g.,double v = Constant.MIN_DIMENSION + rg.nextDouble() * (Constant.MAX_DIMENSION - Constant.MIN_DIMENSION);
and use separate locals per dimension (radius, width, height, legs, top) before constructors. - Constant.java: rename misspelled identifiers
MIN_DIMENTION
/MAX_DIMENTION
→MIN_DIMENSION
/MAX_DIMENSION
and update all references. Also changeDEFAULT_COLOR
from "WHITE" to "white" to match the spec. - Application.java: the array-filling loop can leave
null
entries whenConstant.NUMBER_OF_FIGURES_TO_CREATE
is odd, causing a NullPointerException in the draw loop. Computeint half = size / 2;
and ensure every index is assigned (either enforce an even constant or handle the odd remainder explicitly).
Minor but required style/formatting fixes:
- Remove the leading empty line after the package declaration in all flagged files (Application, AreaCalculator, Circle, ColorSupplier, Colors, Constant, Drawer, Figure, FigureSupplier, FigureWithBottomAndHeight, IsoscelesTrapezoid, Rectangle, RightTriangle, Square).
- ColorSupplier: rename static final
colors
→COLORS
. - Rectangle.java and IsoscelesTrapezoid.java: rename constructor param
heigh
→height
. - Normalize draw() output formatting to match the example: include commas and units before color, e.g.,
..., color: <color>
; add missing units fortop
in IsoscelesTrapezoid; add missing comma before color in Rectangle and RightTriangle; add the missing comma/spacing in Circle. Prefer callinggetArea()
insidedraw()
for freshness.
You’ve got the overall structure right (good separation via interfaces, single suppliers reused). After applying these targeted fixes, the project should compile cleanly and meet the checklist. 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,15 @@ | |||
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.
Style violation — remove leading empty line.
This file begins the class implementation with an empty line directly after the package declaration. This violates the checklist item: "Do not start class or method implementations with an empty line; remove redundant empty lines (see checklist item 1)."
Suggestion: delete the empty line at line 2 so the class declaration follows the package declaration immediately.
for (int i = 0; i < Constant.NUMBER_OF_FIGURES_TO_CREATE / 2; i++) { | ||
figures[i] = fs.getRandomFigure(); | ||
figures[i + Constant.NUMBER_OF_FIGURES_TO_CREATE / 2] = fs.getDefaultFigure(); |
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 runtime error — possible null entries in the figures array when NUMBER_OF_FIGURES_TO_CREATE is odd.
The loop at lines 7-9 fills two positions each iteration using Constant.NUMBER_OF_FIGURES_TO_CREATE / 2
. If Constant.NUMBER_OF_FIGURES_TO_CREATE
is odd, integer division will leave at least one array slot unfilled (null). Later, the for (Figure figure : figures)
loop will call figure.draw()
and throw a NullPointerException.
This violates the requirement in the task: "In the main() method: - The first half of the array elements must be random figures produced by FigureSupplier.getRandomFigure(). - The second half must be default figures produced by FigureSupplier.getDefaultFigure(). - After filling the array, iterate over it and draw each figure (i.e., print its info as specified in 1.4)."
Suggested fixes:
- Compute a local
int half = Constant.NUMBER_OF_FIGURES_TO_CREATE / 2;
and use it when filling the array to improve clarity. - Ensure
Constant.NUMBER_OF_FIGURES_TO_CREATE
is even (preferable), or explicitly handle the odd case (e.g., fill the last element with a default figure) so no array entry remains null before drawing. - Alternatively, fill the first half in one loop and the second half in a separate loop to make intent explicit and robust.
Please update the filling logic so every array element is assigned a non-null Figure before the drawing loop.
@@ -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 has an extra blank line immediately after the package declaration. Remove it so the interface declaration follows the package line without an empty line. This violates the code style checklist: "Do not start class or method implementations with an empty line; remove redundant empty lines (see checklist item 1)."
@@ -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 contains a redundant empty line immediately after the package declaration. The checklist requires: "Do not start class or method implementations with an empty line; remove redundant empty lines." Please remove the empty line so the class starts directly after the package declaration.
|
||
public Circle(String color, double radius) { | ||
super(color); | ||
this.radius = radius; | ||
area = getArea(); | ||
} | ||
|
||
@Override | ||
public double getArea() { | ||
return 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 are redundant empty lines inside the class (lines 5, 11, 16). Per the checklist: "Do not start class or method implementations with an empty line; remove redundant empty lines." Remove these extra empty lines to match the required style.
|
||
public class Rectangle extends FigureWithBottomAndHeight { | ||
|
||
public Rectangle(String color, double bottom, double heigh) { |
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 heigh
(misspelled). Use height
instead to match the field name and correct English spelling. Be consistent with the base class FigureWithBottomAndHeight
, which uses height
as the field name (see FigureWithBottomAndHeight). Renaming improves clarity and follows the project's requirement to use English in code and identifiers.
System.out.println("Figure: rectangle, area: " + area + " sq. units, bottom: " | ||
+ bottom + " units, height: " + height + " 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.
The draw() output concatenation omits a separator/comma before the color, resulting in: "... height: color: ". The task's example output requires comma-separated fields and the pattern: "Figure: rectangle, area: sq. units, bottom: units, height: units, color: ". Please add the missing comma/spacing so output matches the example formatting in the description.
@@ -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.
Leading 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". Please remove the blank line so the class declaration immediately follows the package line. See checklist for guidance.
System.out.println("Figure: right triangle, area: " + area + " sq. units, firstLeg: " | ||
+ firstLeg + " units, secondLeg: " + secondLeg + " 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.
The draw() output is missing a separator/comma before the color resulting in: "... secondLeg: color: ". The checklist and task examples require comma-separated fields and the pattern: "..., secondLeg: units, color: ". Update the print to include the missing comma and unit placement so output matches the required example format.
@@ -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.
Style violation — remove the leading empty line after the package declaration. The checklist states: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines." Please delete the blank line so the class declaration follows the package line immediately. (See Square.java)
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
Decision: Request changes — there are critical blockers that will prevent the project from compiling and tests from passing.
Critical issues to fix:
- Invalid Random API usage in
FigureSupplier
(lines ~12–13):Random#nextDouble(double bound)
does not exist, causing a compile error. Userg.nextDouble()
and scale to the range:double v = Constant.MIN_DIMENSION + rg.nextDouble() * (Constant.MAX_DIMENSION - Constant.MIN_DIMENSION);
. Create separate locals for each needed dimension (e.g.,radius
,width
,height
,firstLeg
,secondLeg
,top
) before constructors, per checklist. - Constant names and values in
Constant.java
: rename misspelledMIN_DIMENTION
/MAX_DIMENTION
→MIN_DIMENSION
/MAX_DIMENSION
(and preferdouble
). Update all usages inFigureSupplier
accordingly. Also setDEFAULT_COLOR = "white"
to match the exact required default figure color. - draw() output formatting mismatches: in
Circle
,Rectangle
,RightTriangle
,IsoscelesTrapezoid
, the printed strings are missing commas/labels and won’t match the required format. Use a consistent template, for example:Figure: circle, area: <area> sq. units, radius: <r> units, color: <color>
(and similarly for other figures: include commas, "units", andcolor: ...
).
Required checklist/style fixes (non-functional but required): remove leading/redundant empty lines after package
and before class/method implementations across many files (Application
, AreaCalculator
, Circle
, ColorSupplier
, Colors
, Constant
, Drawer
, Figure
, FigureSupplier
, FigureWithBottomAndHeight
, Rectangle
, RightTriangle
, Square
, IsoscelesTrapezoid
). Also rename constructor parameters heigh
→ height
in Rectangle
and IsoscelesTrapezoid
for correct English and consistency with the base class.
You’ve got a solid overall structure (good separation into interfaces, suppliers, and figures). Fix the compile error, standardize the constants and draw output, and clean up the blank lines — after that, this should pass. Keep going!
✨ 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,15 @@ | |||
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 so the class starts immediately after the package statement.
@@ -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 begins with an empty line after the package declaration. This violates the checklist item: "Don't begin class or method implementation with an empty line." Remove the blank line so the interface declaration follows the package statement directly.
|
||
public class Circle extends Figure { | ||
private double radius; | ||
|
||
public Circle(String color, double radius) { | ||
super(color); | ||
this.radius = radius; | ||
area = getArea(); | ||
} | ||
|
||
@Override | ||
public double getArea() { | ||
return 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.
Redundant empty lines found (package -> class/constructors/methods). 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 lines at the top and between members so class and methods start without a leading blank line (currently empty lines at lines 2, 5, 11, 16).
System.out.println("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.
The draw() output is missing a separator (comma) before the color label and so doesn't match the required output format. This violates the checklist item: "The draw output must include all relevant properties in a consistent format similar to examples: - figure name - area with "sq. units" - dimensions with "units" - color (verifiable in code by checking strings printed).". Update the println to include commas and the exact labels, for example:
System.out.println("Figure: circle, area: " + area + " sq. units, radius: " + radius + " units, color: " + color);
|
||
import java.util.Random; | ||
|
||
public class ColorSupplier { | ||
private static final Colors[] colors = Colors.values(); | ||
private final Random rg = new Random(); | ||
|
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 lines found. 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 lines after the package declaration and between imports/class members so the file starts and methods don't begin with empty lines.
System.out.println("Figure: right triangle, area: " + area + " sq. units, firstLeg: " | ||
+ firstLeg + " units, secondLeg: " + secondLeg + " 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.
The draw() output is missing a separator (comma) before the color label and the secondLeg
dimension is not followed by the required unit separator. This violates the checklist requirement: "The draw output must include all relevant properties in a consistent format similar to examples: - figure name - area with "sq. units" - dimensions with "units" - color". Update the println to exactly match the examples, for example:
System.out.println("Figure: right triangle, area: " + area + " sq. units, firstLeg: " + firstLeg + " units, secondLeg: " + secondLeg + " units, color: " + color);
This ensures consistent formatting and avoids test mismatches.
@@ -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 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 directly after the package declaration so the class starts immediately after the package statement.
|
||
public class Square extends Figure { | ||
private 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.
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 between the field declaration and the constructor so the constructor does not begin with an empty line.
this.side = side; | ||
area = 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.
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 before the getArea() method so the method does not begin with an empty line.
public double getArea() { | ||
return 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 checklist item: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" Remove the blank line before the draw() method so the method does not begin with an empty 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: Great job implementing the overall architecture — you cleanly separated concerns with Drawer
and AreaCalculator
, used a Figure
base class, kept a single FigureSupplier
in Application
, and correctly filled the array with random and default figures. To improve, please fix a compile blocker in FigureSupplier.java
(lines ~13–18): rg.nextDouble(range)
doesn’t exist; use rg.nextDouble() * range
when generating radius
, width
, height
, firstLeg
, secondLeg
, and top
. Standardize draw() output formatting by adding commas and “units/sq. units” as in the example (e.g., before color
) in Circle
, Rectangle
, RightTriangle
, and IsoscelesTrapezoid
. Clean up redundant leading empty lines per the checklist across multiple files (e.g., Application.java
line 2; AreaCalculator.java
line 2; Circle.java
lines 2, 5, 11, 16; similar in other files) and correct constructor typos heigh
→ height
in Rectangle
and IsoscelesTrapezoid
. Also, avoid the inline magic number in Application
by introducing a local int half = Constant.NUMBER_OF_FIGURES_TO_CREATE / 2;
and reuse it in the loop and indexing. These are quick fixes; your core logic is solid — nicely done and keep it up!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
@@ -0,0 +1,15 @@ | |||
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. 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 (line 2) so the class declaration follows the package line directly.
for (int i = 0; i < Constant.NUMBER_OF_FIGURES_TO_CREATE / 2; i++) { | ||
figures[i] = fs.getRandomFigure(); | ||
figures[i + Constant.NUMBER_OF_FIGURES_TO_CREATE / 2] = fs.getDefaultFigure(); |
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 use Constant.NUMBER_OF_FIGURES_TO_CREATE / 2
directly in the loop condition and again in the index calculation. The checklist states: "All magic numbers in your code should be constants." Create a named local constant (for example int half = Constant.NUMBER_OF_FIGURES_TO_CREATE / 2;
) before the loop and use half
in the loop and index expression to avoid the inline magic number and repeated expression.
@@ -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 violates the checklist item: "Don't begin class or method implementation with an empty line.\nRemove all redundant empty lines, be careful :)" — there is an empty line immediately after the package
statement. Remove that blank line so the interface declaration follows the package line directly.
@@ -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 the class with a blank line 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 this empty line so the class declaration immediately follows the package line.
|
||
public class Circle extends Figure { | ||
private double 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 empty line immediately before the constructor. 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 constructor starts without a leading empty line.
System.out.println("Figure: right triangle, area: " + area + " sq. units, firstLeg: " | ||
+ firstLeg + " units, secondLeg: " + secondLeg + " color: " + color.toLowerCase()); |
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() output concatenation is missing a comma/clear separator before the color field. The task requires printing all figure information in a readable format (see description). Update the println to include a comma and a space before the color and ensure 'units' and 'sq. units' are present. Example:
System.out.println("Figure: right triangle, area: " + area + " sq. units, firstLeg: " + firstLeg + " units, secondLeg: " + secondLeg + " units, color: " + color.toLowerCase());
@@ -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 contains a blank line immediately after the package declaration. Fix per checklist: "Don't begin class or method implementation with an empty line. Remove all redundant empty lines, be careful :)" — remove this empty line so the class declaration follows the package line without a blank line.
|
||
public class Square extends Figure { | ||
private 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.
There is a redundant 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 :)" — remove this blank line so the constructor starts immediately.
this.side = side; | ||
area = 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.
There is an empty line before the getArea() method. 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 method starts without a leading empty line.
public double getArea() { | ||
return 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 before the draw() method. 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 draw() begins immediately.
No description provided.