Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Homework #1316

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Homework #1316

wants to merge 8 commits into from

Conversation

maxymmusiienko
Copy link

made abstract figure class and 5 figures, added color and figure supliers for creating new figures

Rommelua
Rommelua suggested changes Oct 9, 2023
View reviewed changes
Copy link

@Rommelua Rommelua left a 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

src/main/java/core/basesyntax/Area.java Outdated
@@ -0,0 +1,5 @@
package core.basesyntax;

public interface Area {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public interface Area {
public interface AreaCalculator {

src/main/java/core/basesyntax/ColorSupplier.java Outdated
import java.util.Random;

public class ColorSupplier {
private static final String[] COLORS = {"red", "green", "blue", "yellow", "purple", "orange"};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create public enum Color

src/main/java/core/basesyntax/Drawer.java Outdated
@@ -0,0 +1,5 @@
package core.basesyntax;

public interface Drawer {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public interface Drawer {
public interface Drawable {

src/main/java/core/basesyntax/FigureSupplier.java Outdated
private static final Random random = new Random();

public Figure getRandomFigure() {
int choice = random.nextInt(5);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

kozhukhovsky
kozhukhovsky suggested changes Oct 10, 2023
View reviewed changes
src/main/java/core/basesyntax/Color.java Outdated
public enum Color {
RED, GREEN, BLUE, YELLOW, PURPLE, ORANGE, WHITE;

public static Color getRandomColor() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move this method to the ColorSupplier class
and dont use static

src/main/java/core/basesyntax/FigureSupplier.java Outdated
}

public Figure getDefaultFigure() {
return new Circle(Color.WHITE, 10);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move 10 and white to the class constant

src/main/java/core/basesyntax/FigureSupplier.java Outdated
private static final Random random = new Random();

public Figure getRandomFigure() {
final int n = 5;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move 5 to the class constant, like FIGURES_COUNT

okuzan
okuzan suggested changes Oct 11, 2023
View reviewed changes
src/main/java/core/basesyntax/FigureSupplier.java Outdated
}

public Figure getDefaultFigure() {
return new Circle(Color.WHITE, DEFAULT_RADIUS);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make enum value a DEFAULT_COLOR constant

src/main/java/core/basesyntax/FigureSupplier.java Outdated
Comment on lines 22 to 23
return new RightTriangle(color, random.nextDouble() * 10 + 1,
random.nextDouble() * 10 + 1);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract random.nextDouble() * 10 + 1 to a private method, make 10 a constant

src/main/java/core/basesyntax/FigureSupplier.java Outdated
import java.util.Random;

public class FigureSupplier {
private static final Random random = new Random();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private static final Random random = new Random();
private final Random random = new Random();

should be same as color supplier

src/main/java/core/basesyntax/Main.java Outdated
Comment on lines 9 to 19
for (int i = 0; i < 3; i++) {
figures[i] = figureSupplier.getRandomFigure();
}

for (int i = 3; i < 6; i++) {
figures[i] = figureSupplier.getDefaultFigure();
}

for (Figure figure : figures) {
figure.draw();
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be done in one loop. use length/2

src/main/java/core/basesyntax/Main.java Outdated

public class Main {
public static void main(String[] args) {
final int N = 6;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make it a class constant with a more meaningful name :)

@maxymmusiienko maxymmusiienko requested a review from okuzan October 11, 2023 20:45
Ivan95kos
Ivan95kos approved these changes Oct 11, 2023
View reviewed changes
Copy link

@Ivan95kos Ivan95kos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, with a minor comment

src/main/java/core/basesyntax/FigureSupplier.java
private final ColorSupplier colorSupplier = new ColorSupplier();
private final Random random = new Random();

private double getRandomSide() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All private methods should be below public methods

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ivan95kos Ivan95kos Ivan95kos approved these changes

@Rommelua Rommelua Awaiting requested review from Rommelua

@kozhukhovsky kozhukhovsky Awaiting requested review from kozhukhovsky

@okuzan okuzan Awaiting requested review from okuzan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@maxymmusiienko @kozhukhovsky @Rommelua @Ivan95kos @okuzan