Skip to content
Open

Main #2023

Show file tree
Hide file tree
Changes from all 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
54 changes: 54 additions & 0 deletions Lottery.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import java.util.Random;

public class Lottery {

public enum Color {
GREEN, BLUE, PURPLE, WHITE, BLACK, YELLOW, ORANGE
}

public static class ColorSupplier {

Choose a reason for hiding this comment

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

This violates checklist item #3: 'Don't use static methods in your solution'. While the methods inside are not static (which is correct), the class itself should not be static. The classes ColorSupplier, Ball, and Lottery should be regular top-level classes, each in its own file.

private final Random random = new Random();

public String getRandomColor() {
Color[] colors = Color.values();
return colors[random.nextInt(colors.length)].name();
}
}

public static class Ball {
private final String color;
private final int number;

public Ball(String color, int number) {
this.color = color;
this.number = number;
}

@Override
public String toString() {
return "Ball: " + color + ", number: " + number;
}
}

public static class Lottery {

Choose a reason for hiding this comment

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

This nested class has the same name as its outer class (Lottery), which is a major source of confusion. The logic and fields from this class (lines 34-40) should be part of the main public class Lottery defined on line 3. After moving the logic, this nested class should be removed.

private final ColorSupplier colorSupplier = new ColorSupplier();
private final Random random = new Random();
private static final int MAX_NUMBER = 100;

public Ball getRandomBall() {
return new Ball(colorSupplier.getRandomColor(), random.nextInt(MAX_NUMBER + 1));
}
}
Comment on lines +5 to +41

Choose a reason for hiding this comment

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

As mentioned in the previous review, the Color enum, and the ColorSupplier, Ball, and inner Lottery classes should each be in their own separate .java file. It's a standard Java convention to have one top-level public type per file. This nested structure makes the code hard to read and maintain. This also relates to checklist item #3 ('Don't use static methods'), as moving these classes to their own files will mean they no longer need to be static.


private static final int DRAW_COUNT = 3;

public static void main(String[] args) {
Lottery lottery = new Lottery();

System.out.println("Drawing " + DRAW_COUNT + " balls:");

for (int i = 1; i <= DRAW_COUNT; i++) {
System.out.println("Draw number:" + i + lottery.getRandomBall());
}
}
}
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
<<<<<<< HEAD
# mate_test_repository
=======
# jv-lottery

Let's create a simple lottery that will return balls with a random color and random number.
Expand All @@ -15,3 +18,4 @@ To test the program, in the main() method, create three balls using the `getRand
And print information about them in the console.

#### [Try to avoid these common mistakes, while solving task](./checklist.md)
>>>>>>> b478459708f54a5527b8183f5e30d6efea512eec