created a program that generate the array with random/default figures #1771
Conversation
… and print information about them
There was a problem hiding this comment.
Thank you for your hard work on this project! 😊 There are a few critical issues that need to be addressed before we can approve the pull request. Specifically, the printInformation methods in the Ellipse and IsoscelesTrapezoid classes are incorrectly labeling the figures, which impacts the clarity and correctness of the output. Additionally, the default case in the FigureSupplier class's switch statement is unnecessary and could be misleading. Once these issues are resolved, your implementation will be in great shape! Keep up the great work, and feel free to reach out if you have any questions.
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
| public String printInformation() { | ||
| return "Figure: Rectangle, area: " + getArea() |
There was a problem hiding this comment.
The printInformation method incorrectly refers to the figure as a 'Rectangle'. It should be 'Ellipse' to accurately describe the figure type.
| case 3 -> new Ellipse(max1, max2); | ||
| case 4 -> new Square(max1); | ||
| case 5 -> new IsoscelesTrapezoid(max1, max2, max2); | ||
| default -> throw new IllegalArgumentException("Unexpected value"); |
There was a problem hiding this comment.
The default case in the switch statement is unnecessary because the random number generation is controlled by FIRST_MAX_NUMBER and should not exceed this range. Consider removing the default case or handling it in a way that aligns with the expected behavior.
| public String printInformation() { | ||
| return "Figure: RightTriangle, area: " + getArea() |
There was a problem hiding this comment.
The printInformation method incorrectly refers to the figure as a 'RightTriangle'. It should be 'IsoscelesTrapezoid' to accurately describe the figure type.
There was a problem hiding this comment.
Great job on addressing the previous issues and implementing the required features! 🎉 Your code now meets the task requirements, and all classes are correctly implemented with methods for calculating areas and printing information. The FigureSupplier and ColorSupplier classes are well-designed for generating random figures and colors.
While the code is approved, here are a few points to consider for future improvements:
-
EllipseandIsoscelesTrapezoidClasses: You've successfully corrected theprintInformationmethods to label the figures accurately. Keep up the good work in maintaining clarity in your code. -
FigureSupplierClass: Removing the unnecessarydefaultcase and handling unexpected values with anIllegalArgumentExceptionis a robust approach. Ensure to maintain such practices for error handling. -
Task Requirements and Checklist Compliance: Your code adheres to the task requirements and checklist guidelines, such as avoiding static methods and using constants for magic numbers. This is excellent practice for writing clean and maintainable code.
Remember, the decision to approve is to help you move forward with your learning journey. Please review the comments to further refine your skills and address any critical issues in future projects. Keep learning and improving! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| System.out.println(figures[i].printInformation()); | ||
| } else { | ||
| figures[i] = figureSupplier.getDefaultFigure(); | ||
| System.out.println(figures[i].printInformation()); |
| case 3 -> new Ellipse(max1, max2); | ||
| case 4 -> new Square(max1); | ||
| case 5 -> new IsoscelesTrapezoid(max1, max2, max2); | ||
| default -> throw new IllegalArgumentException("Unexpected value"); |
There was a problem hiding this comment.
IllegalState is more applicable here, cuz this method doesn't have arguments, problem can only occur via developer's mistake, misconfiguration of lengths. You can also make case 5 default
|
|
||
| public abstract class Figure implements FigureSquare, FigureInformation { | ||
| private String color; | ||
|
|
There was a problem hiding this comment.
consider creating a constructor for color and calling super() in subclasses
it also prints information about figures