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

Added necessary Classes/Test passed #1508

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/main/java/core/basesyntax/CalculatableArea.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package core.basesyntax;

public interface CalculatableArea {

Choose a reason for hiding this comment

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

Like I've mentioned in previous comment, your Figure is neither FigureArea nor CalculatableArea - but could be for example "AreaCalculator" - please give your classes a meaningful names.

double getArea();
}
22 changes: 22 additions & 0 deletions src/main/java/core/basesyntax/Circle.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package core.basesyntax;

public class Circle extends Figure {
private int radius;

public Circle(int radius, Color color) {
this.radius = radius;
super.color = color;
}

@Override
public double getArea() {
return Math.round(Math.PI * radius * radius);
}

@Override
public String getFigureInfo() {
return "Figure: Circle, area: " + getArea() + " sq. units, radius: "
+ radius + " units, color: " + super.color;
}
/* Wzór na pole koła pi*r*r */
Copy link

Choose a reason for hiding this comment

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

The comment is in Polish and might not be understood by all developers. Consider translating it to English for consistency and clarity.

}
13 changes: 13 additions & 0 deletions src/main/java/core/basesyntax/Color.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package core.basesyntax;

public enum Color {
Copy link

Choose a reason for hiding this comment

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

Consider adding a method to get a random color from the enum. This would be useful for the ColorSupplier class to generate random colors.

RED,
GREEN,
BLUE,
YELLOW,
ORANGE,
CYAN,
WHITE,
BLACK,
LIME
}
14 changes: 14 additions & 0 deletions src/main/java/core/basesyntax/ColorSupplier.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package core.basesyntax;

import java.util.Random;

public class ColorSupplier {

static final Random RANDOM = new Random();

Choose a reason for hiding this comment

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

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


public String getRandomColor() {
int index = RANDOM.nextInt(Color.values().length);
return Color.values()[index].toString();
}

Choose a reason for hiding this comment

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

Suggested change
public String getRandomColor() {
int index = RANDOM.nextInt(Color.values().length);
return Color.values()[index].toString();
}
public Color getRandomColor() {
int index = RANDOM.nextInt(Color.values().length);
return Color.values()[index];
}


}
14 changes: 14 additions & 0 deletions src/main/java/core/basesyntax/Figure.java

Choose a reason for hiding this comment

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

Let's refactor this class to this:

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

    public Figure(Color color) {
        this.color = color;
    }

    public Color getColor() {
        return color;
    }
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package core.basesyntax;

public abstract class Figure implements FigureInfo, CalculatableArea {
private static 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.

Not fixed: #1508 (comment)

protected Color color;

Choose a reason for hiding this comment

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

Suggested change
protected Color color;
private Color color;


public Figure() {
this.color = Color.valueOf(colorSupplier.getRandomColor());
}

public Figure(Color color) {
this.color = color;
}
}
5 changes: 5 additions & 0 deletions src/main/java/core/basesyntax/FigureInfo.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package core.basesyntax;

public interface FigureInfo {
String getFigureInfo();
}
33 changes: 33 additions & 0 deletions src/main/java/core/basesyntax/FigureSupplier.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package core.basesyntax;

import static core.basesyntax.ColorSupplier.RANDOM;

public class FigureSupplier {
private static final int FIGURE_COUNT = 5;
private static final int MIN_LENGTH = 2;
private static final int MAX_LENGTH = 10;

public Figure getRandomFigure() {
int ran = RANDOM.nextInt(FIGURE_COUNT);
int a = RANDOM.nextInt(MAX_LENGTH - MIN_LENGTH + 1) + MIN_LENGTH;

Choose a reason for hiding this comment

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

I don't understand what is happening here - why not just use RANDOM.nextInt(MAX_LENGTH + 1)? Do we need MIN_LENGTH?

int b = RANDOM.nextInt(MAX_LENGTH - MIN_LENGTH + 1) + MIN_LENGTH;
int c = RANDOM.nextInt(MAX_LENGTH - MIN_LENGTH + 1) + MIN_LENGTH;

Choose a reason for hiding this comment

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

Not fixed: #1508 (comment)

switch (ran) {
case (0):
return new Square(a);

Choose a reason for hiding this comment

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

In all cases you should pass random size and random color, so create and use ColorSupplier, For example like this (You also need random here):

private static final ColorSupplier colorSupplier = new ColorSupplier();
private static final Random random = new Random();
private static final int FIGURE_COUNT = 5;
private static final int MIN_LENGTH = 2;
private static final int MAX_LENGTH = 10;

case (1):
return new Rectangle(a, b);
case (2):
return new RightTriangle(a, b);
case (3):
return new Circle(a, Color.WHITE);

Choose a reason for hiding this comment

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

Why you allow to pass color in constructor for Circle but not the rest of figures?

default:
return new IsoscelesTrapezoid(a, b, c);
}

}

public Figure getDefaultFigure() {
return new Circle(10, Color.WHITE);
}
}
8 changes: 0 additions & 8 deletions src/main/java/core/basesyntax/HelloWorld.java

This file was deleted.

33 changes: 33 additions & 0 deletions src/main/java/core/basesyntax/IsoscelesTrapezoid.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package core.basesyntax;

public class IsoscelesTrapezoid extends Figure {
private int firstBase;
private int secondBase;
private int heigh;
Copy link

Choose a reason for hiding this comment

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

There is a typo in the variable name 'heigh'. It should be 'height'. This will improve code readability and prevent potential confusion.


public IsoscelesTrapezoid(int firstBase, int secondBase, int heigh) {
this.firstBase = firstBase;
this.secondBase = secondBase;
this.heigh = heigh;
}

public IsoscelesTrapezoid(int firstBase, int secondBase, int heigh, Color color) {
this.firstBase = firstBase;
this.secondBase = secondBase;
this.heigh = heigh;
super.color = color;
}

@Override
public double getArea() {
return (firstBase * secondBase) * heigh / 2;

Choose a reason for hiding this comment

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

Suggested change
return (firstBase * secondBase) * heigh / 2;
return (firstBase + secondBase) * heigh / 2;

}

@Override
public String getFigureInfo() {
return "Figure: IsoscelesTrapezoid, area: " + getArea() + " sq. units, firstBase: "
Copy link

Choose a reason for hiding this comment

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

The method 'getFigureInfo' should use consistent formatting for the output string. Consider adding commas or semicolons between the attributes for better readability.

+ firstBase + " secondBase "
+ secondBase + " heigh " + heigh + " units, color: " + super.color;
}
/* Wzór na pole trapezu równoramiennego (a+b)*h /2 */
Copy link

Choose a reason for hiding this comment

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

The comment on line 26 is in Polish. It should be translated to English to maintain consistency and ensure that all developers can understand it.

}
15 changes: 15 additions & 0 deletions src/main/java/core/basesyntax/Main.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package core.basesyntax;

public class Main {
public static void main(String[] args) {
FigureSupplier figureIndex = new FigureSupplier();
Copy link

Choose a reason for hiding this comment

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

The variable name 'figureIndex' is misleading. It suggests an index or position rather than a supplier. Consider renaming it to 'figureSupplier' for clarity.

Figure[] figures = new Figure[6];
for (int i = 0; i < 6; i++) {
figures[i] = figureIndex.getRandomFigure();

}
for (int i = 0; i < 6; i++) {
System.out.println(figures[i].getFigureInfo());
}
}
}
29 changes: 29 additions & 0 deletions src/main/java/core/basesyntax/Rectangle.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package core.basesyntax;

public class Rectangle extends Figure {
private int base;
Copy link

Choose a reason for hiding this comment

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

The variable names 'base' and 'side' are not very descriptive for a rectangle. Consider using 'width' and 'height' for clarity.

private int side;

public Rectangle(int base, int side) {
this.base = base;
this.side = side;
}

Choose a reason for hiding this comment

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

This constructor is redundant, please remove it from all classes that extends Figure (constructors without color)


public Rectangle(int base, int side, Color color) {
this.base = base;
this.side = side;
super.color = color;
}

Choose a reason for hiding this comment

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

It should be like this:

Suggested change
public Rectangle(int base, int side, Color color) {
this.base = base;
this.side = side;
super.color = color;
}
public Rectangle(int base, int side, Color color) {
super(color);
this.base = base;
this.side = side;
}

Please do it in all classes that extends Figure


@Override
public double getArea() {
return base * side;
}

@Override
public String getFigureInfo() {
return "Figure: Rectangle, area: " + getArea() + " sq. units, base: " + base
+ " side " + side + " units, color: " + super.color;
}
/* Wzór na pole Prostokąta a*b */
Copy link

Choose a reason for hiding this comment

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

The comment is in Polish, which might not be understandable to all developers. Consider translating it to English.

}
29 changes: 29 additions & 0 deletions src/main/java/core/basesyntax/RightTriangle.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package core.basesyntax;

public class RightTriangle extends Figure {
private int base;
private int heigh;
Copy link

Choose a reason for hiding this comment

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

There is a typo in the variable name 'heigh'. It should be 'height'. This typo is repeated in the constructor and other parts of the code.


public RightTriangle(int base, int heigh) {
this.base = base;
this.heigh = heigh;
}

public RightTriangle(int base, int heigh, Color color) {
this.base = base;
this.heigh = heigh;
super.color = color;
}

@Override
public double getArea() {
return (base * heigh) / 2;
Copy link

Choose a reason for hiding this comment

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

The calculation for the area should be cast to double to ensure precision, as integer division may lead to loss of precision. Consider using '(double) base * heigh / 2' to ensure the result is a double.

}

@Override
public String getFigureInfo() {
return "Figure: RightTriangle, area: " + getArea() + " sq. units, base: "
+ base + " heigh " + heigh + " units, color: " + super.color;
}
/* Wzór na pole trójkąta prostokątnego (a*h)/2 */
}
27 changes: 27 additions & 0 deletions src/main/java/core/basesyntax/Square.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package core.basesyntax;

public class Square extends Figure {
private int side;
Copy link

Choose a reason for hiding this comment

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

The 'side' attribute should be validated to ensure it is a positive integer. Consider adding a check in the constructor to throw an IllegalArgumentException if 'side' is less than or equal to zero.


public Square(int side) {
this.side = side;
}

public Square(int side, Color color) {
this.side = side;
super.color = color;
}

@Override
public double getArea() {
return side * side;
}

@Override
public String getFigureInfo() {
return "Figure: Square, area: " + getArea() + " sq. units, side: "
+ side + " units, color: " + super.color;
}
/* Wzór na pole kwadratu a*a */
Copy link

Choose a reason for hiding this comment

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

The comment on line 21 is in Polish. Consider translating it to English for consistency and to ensure that all developers can understand it.


}
Loading