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

make task #1311

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

make task #1311

wants to merge 11 commits into from

Conversation

Paradoxallist
Copy link

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.

https://mate-academy.github.io/jv-program-common-mistakes/java-core/abstract-class-interface/oop-advanced

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

Comment on lines 7 to 9
this.radius = radius;
setColor(color);
getArea();
Copy link

Choose a reason for hiding this comment

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

Suggested change
this.radius = radius;
setColor(color);
getArea();
super(color);
this.radius = radius;


public class ColorSupplier {
public String getRandomColor() {
Random rand = new Random();
Copy link

Choose a reason for hiding this comment

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

image

src/main/java/core/basesyntax/ColorSupplier.java Outdated Show resolved Hide resolved
Comment on lines 18 to 20
System.out.println("Circle{" + "radius=" + radius
+ ", color='" + getColor() + '\'' + ", area=" + getArea() + '}');
return "";

Choose a reason for hiding this comment

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

Suggested change
System.out.println("Circle{" + "radius=" + radius
+ ", color='" + getColor() + '\'' + ", area=" + getArea() + '}');
return "";
return "Circle{" + "radius=" + radius
+ ", color='" + getColor() + '\'' + ", area=" + getArea() + '}';

Choose a reason for hiding this comment

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

behaviour - we can obtain the area of any figure and are able to draw it. To 'draw' means to print out all information about a figure using System.out.println() (you shouldn't override toString() method for this).


public class ColorSupplier {
private final Random rand = new Random();
private final String[] colors = new String[]{"white", "black", "blue", "red"};

Choose a reason for hiding this comment

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

make Color enum


public Figure getRandomFigure() {
String randomFigure = listFigure[random.nextInt(listFigure.length)];
int randomRange = 100;

Choose a reason for hiding this comment

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

should be class level constant.

randomRange is misleading name

Comment on lines +22 to +25
System.out.println("IsoscelesTrapezoid{" + "upSide=" + upSide
+ ", downSide=" + downSide
+ ", height=" + height + ", color='"
+ getColor() + '\'' + ", area=" + getArea() + '}');

Choose a reason for hiding this comment

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

behaviour - we can obtain the area of any figure and are able to draw it. To 'draw' means to print out all information about a figure using System.out.println() (you shouldn't override toString() method for this).

figures[2] = new Circle("white", 10);
figures[3] = new Square("black", 5);
for (Figure figure : figures) {
figure.toString();

Choose a reason for hiding this comment

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

consider adding interface instead of overriding toString

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.

Comment on lines 4 to 5
white,
black,

Choose a reason for hiding this comment

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

uppercase

return color;
}

public abstract void draw();

Choose a reason for hiding this comment

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

image

import java.util.Random;

public class FigureSupplier {
private final int maxNumber = 100;

Choose a reason for hiding this comment

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

image

Comment on lines 4 to 8
Circle,
Square,
Rectangle,
RightTriangle,
IsoscelesTrapezoid

Choose a reason for hiding this comment

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

uppercase

public class Circle extends Figure {
private final int radius;

public Circle(Color color,int radius) {

Choose a reason for hiding this comment

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

There should be a space after the comma in the parameters list of the constructor.

package core.basesyntax;

public class Square extends Figure {
private final double side;

Choose a reason for hiding this comment

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

this fields is marked as 'final' which means that it can't be changed after it's initialized. This is okay if you will never need to change this value, but if you want your objects to be mutable, you should remove the 'final' keyword.

Copy link
Author

Choose a reason for hiding this comment

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

This object is designed to be immutable, which does not change after it is created. In fact, I should have made the shape classes themselves final (Circle, Square and others).

import java.util.Random;

public class FigureSupplier {
public static final int MAX_NUMBER = 100;

Choose a reason for hiding this comment

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

The MAX_NUMBER constant could be private as it's not used outside of this class.


public class FigureSupplier {
public static final int MAX_NUMBER = 100;
private final ColorSupplier colorSupplier = new ColorSupplier();

Choose a reason for hiding this comment

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

The colorSupplier object could be static because its method is stateless, and it would be more efficient to have only one instance of ColorSupplier for all FigureSupplier objects.

}

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

Choose a reason for hiding this comment

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

image

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

Choose a reason for hiding this comment

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

redundant indent line

package core.basesyntax;

public class RightTriangle extends Figure {
private final double firstLeg;

Choose a reason for hiding this comment

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

this fields is marked as 'final' which means that it can't be changed after it's initialized. This is okay if you will never need to change this value, but if you want your objects to be mutable, you should remove the 'final' keyword.

package core.basesyntax;

public class IsoscelesTrapezoid extends Figure {
private final double upSide;

Choose a reason for hiding this comment

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

this fields is marked as 'final' which means that it can't be changed after it's initialized. This is okay if you will never need to change this value, but if you want your objects to be mutable, you should remove the 'final' keyword.

package core.basesyntax;

public abstract class Figure implements AreaCalculator, FigureDrawer {
private final Color color;

Choose a reason for hiding this comment

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

this field is marked as 'final' which means that it can't be changed after it's initialized. This is okay if you will never need to change this value, but if you want your objects to be mutable, you should remove the 'final' keyword.

package core.basesyntax;

public class Circle extends Figure {
private final int radius;

Choose a reason for hiding this comment

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

this field is marked as 'final' which means that it can't be changed after it's initialized. This is okay if you will never need to change this value, but if you want your objects to be mutable, you should remove the 'final' keyword.

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