-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add classes for figures with area calculation and drawing #1713
base: master
Are you sure you want to change the base?
Conversation
…tity abstract class
…htTriangle, Square) with area calculation and drawing functionality
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.
Great job on this pull request! You've followed the task requirements and checklist well. There were a few minor issues pointed out, but they don't significantly impact the functionality of your code. Keep up the good work! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
|
||
public class ColorSupplier { | ||
public String getRandomColor() { | ||
Random random = 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.
Static methods are in general a bad practice. Let's better create an instance of a class which method you want to call. You should create an instance of the Random
class at the class level, not inside the method.
public String getRandomColor() { | ||
Random random = new Random(); | ||
Colors[] colors = Colors.values(); | ||
return colors[random.nextInt(colors.length)].toString(); |
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.
Don't use toString()
or String.valueOf()
(it will call toString()
under the hood) for getting the String
representation of enum constants. Use name()
method instead.
String color = colorSupplier.getRandomColor(); | ||
switch (figureIndex) { | ||
case 0: | ||
return new Square(rand.nextDouble(100) + 1, 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.
Creating a figure, don't pass expressions in the constructor. Create separate variables and pass them on for better code readability. (Checklist point: Creating a figure, don't pass expressions in the constructor.)
case 0: | ||
return new Square(rand.nextDouble(100) + 1, color); | ||
case 1: | ||
return new Rectangle(rand.nextDouble(100) + 1, rand.nextDouble(100) + 1, 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.
Creating a figure, don't pass expressions in the constructor. Create separate variables and pass them on for better code readability. (Checklist point: Creating a figure, don't pass expressions in the constructor.)
case 1: | ||
return new Rectangle(rand.nextDouble(100) + 1, rand.nextDouble(100) + 1, color); | ||
case 2: | ||
return new Circle(rand.nextDouble(100) + 1, 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.
Creating a figure, don't pass expressions in the constructor. Create separate variables and pass them on for better code readability. (Checklist point: Creating a figure, don't pass expressions in the constructor.)
case 2: | ||
return new Circle(rand.nextDouble(100) + 1, color); | ||
case 3: | ||
return new RightTriangle(rand.nextDouble(100) + 1, rand.nextDouble(100) + 1, 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.
Creating a figure, don't pass expressions in the constructor. Create separate variables and pass them on for better code readability. (Checklist point: Creating a figure, don't pass expressions in the constructor.)
case 4: | ||
return new IsoscelesTrapezoid(rand.nextDouble(100) + 1, | ||
rand.nextDouble(100) + 1, | ||
rand.nextDouble(100) + 1, 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.
Creating a figure, don't pass expressions in the constructor. Create separate variables and pass them on for better code readability. (Checklist point: Creating a figure, don't pass expressions in the constructor.)
} | ||
|
||
public Figure getDefaultFigure() { | ||
return new Circle(10, Colors.WHITE.toString()); |
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.
Don't use toString()
or String.valueOf()
(it will call toString()
under the hood) for getting the String
representation of enum constants. Use name()
instead. (Checklist point: Use name() for getting String representation of enum constants.)
No description provided.