-
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
created figures and interfaces with calculation and drawing methods #1326
base: master
Are you sure you want to change the base?
Conversation
private final String[] colors = {"red", "green", "blue", "yellow", "purple", "orange", "pink"}; | ||
|
||
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.
consider making class-level field
@@ -0,0 +1,5 @@ | |||
package core.basesyntax; | |||
|
|||
interface DrawFigure { |
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.
No need to tight interface name for figure only
interface DrawFigure { | |
interface Drawable { |
Random random = new Random(); | ||
ColorSupplier colorSupplier = new ColorSupplier(); | ||
|
||
int figureType = random.nextInt(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.
avoid using magic numbers
consider making it constant
|
||
public class FigureSupplier { | ||
public Figure getRandomFigure() { | ||
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.
consider making class-level field
Correct Task !!! |
public Figure getRandomFigure() { | ||
Random random = new Random(); | ||
ColorSupplier colorSupplier = new ColorSupplier(); |
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.
can be class-level private final variable
@@ -3,11 +3,12 @@ | |||
import java.util.Random; | |||
|
|||
public class FigureSupplier { | |||
private 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.
could be final
@@ -29,7 +30,7 @@ public Figure getRandomFigure() { | |||
int topBase = random.nextInt() * 10; | |||
int bottomBase = random.nextInt() * 10; | |||
int heightTrapezoid = random.nextInt() * 10; | |||
return new IsoscelesTrapezoid(color,topBase, bottomBase, heightTrapezoid); | |||
return new IsoscelesTrapezoid(color, topBase, bottomBase, heightTrapezoid); | |||
|
|||
default: | |||
return null; |
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.
@@ -3,10 +3,10 @@ | |||
import java.util.Random; | |||
|
|||
public class ColorSupplier { | |||
private Random random = new Random(); | |||
private final String[] colors = {"red", "green", "blue", "yellow", "purple", "orange", "pink"}; |
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.
could be private static final
@@ -3,10 +3,10 @@ | |||
import java.util.Random; | |||
|
|||
public class ColorSupplier { | |||
private 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.
could be private final
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.
Before you create PR with your homework, you need to go through the checklist under the task and correct all the points described there. The mentor will not check the work until the checklist points are corrected
interface AreaCalculator { | ||
|
||
double calculateArea(); |
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.
class ColorSupplier { | ||
|
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.
private static final String[] colors = {"red", "green", "blue", "yellow", "purple", | ||
"orange", "pink"}; |
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.
Let's remove this array and create enum Color
(as a separate class)
|
||
public Figure getRandomFigure() { | ||
|
||
final int figureType = random.nextInt(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.
package core.basesyntax; | ||
|
||
enum Colors { | ||
RED, GREEN, WHITE, BLACK, BLUE; |
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.
Better write in stack instead line
case 0: | ||
return new Square(color, sideOrRadius); | ||
case 1: | ||
return new Circle(color, sideOrRadius); | ||
case 2: | ||
double width = random.nextDouble() * 10; | ||
double height = random.nextDouble() * 10; | ||
return new Rectangle(color, (int) width, (int) height); | ||
case 3: | ||
double firstLeg = random.nextDouble() * 10; | ||
double secondLeg = random.nextDouble() * 10; | ||
return new RightTriangle(color, (int) secondLeg, (int) firstLeg); | ||
case 4: | ||
int topBase = random.nextInt() * 10; | ||
int bottomBase = random.nextInt() * 10; | ||
int heightTrapezoid = random.nextInt() * 10; | ||
return new IsoscelesTrapezoid(color, topBase, bottomBase, heightTrapezoid); |
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.
For every case you can create own method and you have a lot of duplicate code with your random values
} | ||
|
||
public Figure getDefaultFigure() { | ||
return new Circle("white", 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.
Use constant for parameters
No description provided.