Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
9 changes: 8 additions & 1 deletion src/main/java/core/basesyntax/Application.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

public class Application {
public static void main(String[] args) {
// create three balls using class Lottery and print information about them in console
Lottery lottery = new Lottery();
Ball ball1 = lottery.getRandomBall();

Choose a reason for hiding this comment

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

Please use for loop.
if you have 100 balls, you won't need to create 100 lines of code

Ball ball2 = lottery.getRandomBall();
Ball ball3 = lottery.getRandomBall();

System.out.println(ball1);
System.out.println(ball2);
System.out.println(ball3);

Choose a reason for hiding this comment

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

Instead of creating multiple Ball instances separately, consider using a for-loop to create and print the balls. This will make the code more concise and easier to maintain if you need to generate a different number of balls in the future. Refer to the checklist about using a for loop for creating several objects of the same class.

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

public class Ball {
private Color color;

Choose a reason for hiding this comment

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

Suggested change
private Color color;
private String color;

private int number;

public Ball(Color color, int number) {

Choose a reason for hiding this comment

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

Suggested change
public Ball(Color color, int number) {
public Ball(String color, int number) {

this.number = number;
this.color = color;
}

public int getNumber() {
return number;
}

public void setNumber(int number) {
this.number = number;
}

@Override
public String toString() {
return "Ball{"
+ "color=" + color.name()
+ ", number=" + number
+ '}';
}
}
5 changes: 5 additions & 0 deletions src/main/java/core/basesyntax/Color.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package core.basesyntax;

public enum Color {
RED, GREEN, BLUE, YELLOW, ORANGE, BLACK, WHITE

Choose a reason for hiding this comment

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

Suggested change
RED, GREEN, BLUE, YELLOW, ORANGE, BLACK, WHITE
RED,
GREEN,
BLUE,
YELLOW,
ORANGE,
BLACK,
WHITE

Choose a reason for hiding this comment

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

not fixed

}
8 changes: 6 additions & 2 deletions src/main/java/core/basesyntax/ColorSupplier.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
package core.basesyntax;

import java.util.Random;

public class ColorSupplier {
public String getRandomColor() {
return null;
public Color getRandomColor() {

Choose a reason for hiding this comment

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

Suggested change
public Color getRandomColor() {
public String getRandomColor() {

Choose a reason for hiding this comment

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

not fixed

Random random = new Random();

Choose a reason for hiding this comment

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

Creating a new instance of 'Random' each time the method is called can be inefficient if the method is called multiple times. Consider making 'random' a class-level instance variable.

Choose a reason for hiding this comment

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

fix comment

Color[] colors = Color.values();

Choose a reason for hiding this comment

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

you can create it as class-level variable

return colors[random.nextInt(colors.length)];
}
}
12 changes: 12 additions & 0 deletions src/main/java/core/basesyntax/Lottery.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package core.basesyntax;

import java.util.Random;

class Lottery {

Choose a reason for hiding this comment

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

Classes must have access modifiers; otherwise, the default will be used, which isn't a good practice. Consider adding an access modifier to the Lottery class.

Choose a reason for hiding this comment

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

Suggested change
class Lottery {
public class Lottery {

public Ball getRandomBall() {
Random random = new Random();

Choose a reason for hiding this comment

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

The instance of Random could be a class-level field instead of being created every time the 'getRandomBall' method is called. This would be more efficient as it avoids unnecessary object creation.

Choose a reason for hiding this comment

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

fix comment

Color color = new ColorSupplier().getRandomColor();

Choose a reason for hiding this comment

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

Creating a new instance of ColorSupplier every time may be inefficient if the getRandomColor method does not depend on instance state. Consider making ColorSupplier a class-level field or its method static if there's no state.

Choose a reason for hiding this comment

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

fix comment

int number = random.nextInt(101);

Choose a reason for hiding this comment

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

Magic numbers should be avoided and replaced with named constants for better code readability. Consider creating a constant for the number 101, giving it a meaningful name that explains what it represents.

Choose a reason for hiding this comment

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

fix comment

Choose a reason for hiding this comment

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

not fixed

return new Ball(color, number);

Choose a reason for hiding this comment

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

Suggested change
return new Ball(color, number);
String randomColor = colorSupplier.getRandomColor();
return new Ball(randomColor, number);

}
}
Loading