jv-oop-advanced homework #1297
jv-oop-advanced homework #1297
Conversation
| } | ||
| } | ||
| for (Figure figure : figures) { | ||
| figure.getArea(); |
There was a problem hiding this comment.
Why did you make this call and then ignore result of it?
There was a problem hiding this comment.
get.Area is already implemented in a print method so it has no sense to call it 2 times, don't know why I did it)
|
|
||
| @Override | ||
| public double getArea() { | ||
| return (side * height) / 2; |
There was a problem hiding this comment.
integer division in floating-point context .
Please pay attention to this article.
There was a problem hiding this comment.
Should I give height field a double value by default or should I cast it to double in a getArea method? Hope I changed it in a right way
|
|
||
| @Override | ||
| public double getArea() { | ||
| return ((side + side) / 2) * height; |
There was a problem hiding this comment.
integer division in floating-point context .
Please pay attention to this article.
| } | ||
|
|
||
| public Figure getDefaultFigure() { | ||
| return new Circle(Color.WHITE, 10); |
There was a problem hiding this comment.
All magic numbers in your code should be constants.
Please see this article to learn about constant fields and their naming requirements.
| @@ -0,0 +1,5 @@ | |||
| package core.basesyntax; | |||
|
|
|||
| public interface GetArea { | |||
| package core.basesyntax; | ||
|
|
||
| public interface Print { | ||
| void print(); |
There was a problem hiding this comment.
I think figure should draw instead of print. In the task: To 'draw' means to print.
Lets rename it to Drawable with void draw().
| int index = random.nextInt(Color.values().length); | ||
| return Color.values()[index]; |
There was a problem hiding this comment.
I think you can make Color.values() as separate class field.
| double sideD = side; | ||
| double heightD = height; |
| package core.basesyntax; | ||
|
|
||
| public class Circle extends Figure { | ||
| private Color color; |
There was a problem hiding this comment.
You already have Color color field in Figure class.
There was a problem hiding this comment.
It has no sence, cuz it has to be inharited from Figure. Changed that everywhere
| double sideD = side; | ||
| double heightD = height; |
| private Random random = new Random(); | ||
|
|
||
| public Color getRandomColor() { | ||
| int index = random.nextInt(Color.values().length); |
There was a problem hiding this comment.
Minor: you already have the field which holds colors, you can reuse it here
| static final int FIGURE_AMOUNT = 5; | ||
| static final int FIGURE_PARAMETERS_MAX = 20; | ||
| static final Color DEFAULT_COLOR = Color.WHITE; | ||
| static final int DEFAULT_RADIUS = 10; |
There was a problem hiding this comment.
Don't forget to make these fields private
| case 0: { | ||
| return new Circle(colorSupplier.getRandomColor(), randomFigureParameter); | ||
| } | ||
| case 1: { | ||
| return new Square(colorSupplier.getRandomColor(), randomFigureParameter); | ||
| } | ||
| case 2: { | ||
| return new Rectangle(colorSupplier.getRandomColor(), | ||
| randomFigureParameter, randomFigureParameter); | ||
| } | ||
| case 3: { | ||
| return new RightTriangle(colorSupplier.getRandomColor(), | ||
| randomFigureParameter, randomFigureParameter); | ||
| } | ||
| default: { | ||
| return new IsoscelesTrapezoid(colorSupplier.getRandomColor(), | ||
| randomFigureParameter, randomFigureParameter); |
There was a problem hiding this comment.
since each figure needs a color you can extract colorSupplier.getRandomColor() to a separate variable and reuse
| case 0: { | ||
| return new Circle(colorSupplier.getRandomColor(), randomFigureParameter); | ||
| } | ||
| case 1: { | ||
| return new Square(colorSupplier.getRandomColor(), randomFigureParameter); | ||
| } | ||
| case 2: { | ||
| return new Rectangle(colorSupplier.getRandomColor(), | ||
| randomFigureParameter, randomFigureParameter); | ||
| } | ||
| case 3: { | ||
| return new RightTriangle(colorSupplier.getRandomColor(), | ||
| randomFigureParameter, randomFigureParameter); | ||
| } | ||
| default: { | ||
| return new IsoscelesTrapezoid(colorSupplier.getRandomColor(), | ||
| randomFigureParameter, randomFigureParameter); |
There was a problem hiding this comment.
Your method is not fully random. For example, your Rectangle will always be a square since you pass the same randomFigureParameter there. So sizes of rectangle will always be equal. Please refactor this part
No description provided.