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.

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 an example of a figure factory #1330

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

Conversation

AShtuka
Copy link

@AShtuka AShtuka commented Oct 18, 2023

No description provided.

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

package core.basesyntax;

public class Circle extends Figure {
private double radius = 0;

Choose a reason for hiding this comment

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

Suggested change
private double radius = 0;
private double radius;

Comment on lines 6 to 8
Circle() {
super("circle", Color.WHITE);
}

Choose a reason for hiding this comment

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

this is unexpected

super("circle", Color.WHITE);
}

Circle(Color color) {

Choose a reason for hiding this comment

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

public

Comment on lines 19 to 23
if (radius > 0) {
this.radius = radius;
} else {
System.out.println("Circle radius can't be negative or equal to zero!");
}

Choose a reason for hiding this comment

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

Suggested change
if (radius > 0) {
this.radius = radius;
} else {
System.out.println("Circle radius can't be negative or equal to zero!");
}
this.radius = radius;

@@ -0,0 +1,5 @@
package core.basesyntax;

public interface DrawAble {

Choose a reason for hiding this comment

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

Suggested change
public interface DrawAble {
public interface Drawable {

package core.basesyntax;

public interface AreaCalculator {
double area();

Choose a reason for hiding this comment

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

Suggested change
double area();
double getArea();

public class FigureSupplier {
private final Random random = new Random();
private final ColorSupplier colorSupplier = new ColorSupplier();
private final int figureAmount = 5;

Choose a reason for hiding this comment

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

image


public Figure getDefaultFigure() {
Circle circle = new Circle();
circle.setRadius(10);

Choose a reason for hiding this comment

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

image

@AShtuka AShtuka requested a review from Rommelua October 19, 2023 17:56
public class Circle extends Figure {
private double radius;

public Circle() {

Choose a reason for hiding this comment

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

KISS - if you dont need default constructor - just delete it

Comment on lines 41 to 43
Circle circle = new Circle();
circle.setRadius(DEFAULT_RADIUS);
return circle;

Choose a reason for hiding this comment

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

Suggested change
Circle circle = new Circle();
circle.setRadius(DEFAULT_RADIUS);
return circle;
return new Circle(DEFAULT_RADIUS);


public class FiguresExpo {
public static void main(String[] args) {
Figure[] figures = new Figure[10];

Choose a reason for hiding this comment

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

extract it to variable

FigureSupplier figureSupplier = new FigureSupplier();

for (int i = 0; i < figures.length; i++) {
if (i < 5) {

Choose a reason for hiding this comment

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

what if we would have only 2 figures? think about how to make this code more universal

}
}

for (Figure figure: figures) {

Choose a reason for hiding this comment

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

no need to create another loop, can do this in the loop above

…plier class/ create constructor in FigureSupplier for set figures array size
@AShtuka AShtuka requested a review from kerrrusha October 20, 2023 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants