jv-oop-advanced the task is completed #1720
Conversation
There was a problem hiding this comment.
Great job on implementing the Color enum correctly! While there are some suggestions for improving code readability and naming conventions, they don't critically impact the functionality. Keep up the good work and consider the feedback for future enhancements! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| @@ -0,0 +1,26 @@ | |||
| package core.basesyntax; | |||
|
|
|||
| public class Circle extends Default { | |||
There was a problem hiding this comment.
The class Circle should not extend Default. Consider using an interface to define the behavior instead of using an abstract class.
|
|
||
| @Override | ||
| public double getArea() { | ||
| return size * size; |
There was a problem hiding this comment.
The formula for the area of a circle is incorrect. It should be Math.PI * size * size to calculate the area using the radius.
|
|
||
| public class ColorSupplier { | ||
| public String getRandomColor() { | ||
| int index = new Random().nextInt(Color.values().length); |
There was a problem hiding this comment.
Consider creating a single instance of Random as a class-level variable instead of creating a new instance each time the method is called. This aligns with the checklist item: 'Think about which variables should be local in the method and which should be class-level'.
| @@ -0,0 +1,29 @@ | |||
| package core.basesyntax; | |||
|
|
|||
| public class Default implements Figure { | |||
There was a problem hiding this comment.
The class name 'Default' is a verb, which is not recommended for class names. Consider using a noun that better describes the purpose of this class.
| public double getArea() { | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
Returning 0 as the area might not be meaningful. Consider implementing this method to return a more appropriate value or throw an exception if this class is not supposed to have an area.
| } | ||
|
|
||
| @Override | ||
| public double getProperty() { |
There was a problem hiding this comment.
The method name getProperty() is not descriptive. Consider renaming it to something more meaningful like getPerimeter() if it calculates the perimeter, or another appropriate name.
|
|
||
| @Override | ||
| public double getProperty() { | ||
| return 0.5 * firstLeg * secondLeg; |
There was a problem hiding this comment.
The calculation here is correct for the area of a right triangle, but it should be in the getArea() method. Ensure that getProperty() is used for a different purpose or rename it.
| @@ -0,0 +1,27 @@ | |||
| package core.basesyntax; | |||
|
|
|||
| public class Square extends Default { | |||
There was a problem hiding this comment.
The class name 'Square' is appropriate as it is a noun and represents a shape. However, ensure that the class 'Default' is correctly used and named, as it might not be descriptive enough.
| public class Square extends Default { | ||
| private double size; | ||
|
|
||
| public Square(String color,double size) { |
There was a problem hiding this comment.
Consider adding a space after the comma in the constructor parameters for better readability: 'String color, double size'.
| } | ||
|
|
||
| @Override | ||
| public double getProperty() { |
There was a problem hiding this comment.
The method name 'getProperty' is not very descriptive. Consider renaming it to something more specific like 'getPerimeter' to better reflect its purpose.
No description provided.