create and implement class Ball and Lottery; create enum Color; imple… #1576
Conversation
…ments class ColorSupplier; implements main() in Application class
There was a problem hiding this comment.
Great job on your code submission! 👍 It looks like you've followed the checklist thoroughly, and there are no critical issues that impact the functionality or security of your code. Remember to always consider the scope of your methods and fields, and whether they should be static or not. Keep up the good work, and continue paying attention to best practices for a clean and maintainable codebase! 😊👨💻
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| import java.util.Random; | ||
|
|
||
| public class ColorSupplier { | ||
| private static final int COLOR_COUNT = Color.values().length; |
There was a problem hiding this comment.
Using static fields for constants is a good practice. However, be cautious with static fields that are not constants, like random in line 7. It's better to avoid static methods and fields unless they are constants or there is a specific reason to use them.
| private static final int COLOR_COUNT = Color.values().length; | ||
| private Random random = new Random(); | ||
|
|
||
| public String getRandomColor() { |
There was a problem hiding this comment.
According to Java best practices, it's recommended to use access modifiers explicitly. Consider defining the access level for the getRandomColor method explicitly if you have a specific intention for package-private access level.
| Ball ball = lottery.getRandomBall(); | ||
| System.out.println(ball); |
There was a problem hiding this comment.
| Ball ball = lottery.getRandomBall(); | |
| System.out.println(ball); | |
| System.out.println(lottery.getRandomBall()); |
| return color; | ||
| } | ||
|
|
||
| public void setColor(String color) { |
There was a problem hiding this comment.
remove methods that you don't use
| import java.util.Random; | ||
|
|
||
| public class ColorSupplier { | ||
| private static final int COLOR_COUNT = Color.values().length; |
There was a problem hiding this comment.
| private static final int COLOR_COUNT = Color.values().length; | |
| private static final int COLORS = Color.values(); |
or your suggestion of name
|
|
||
| public String getRandomColor() { | ||
| return null; | ||
| int colorNumber = random.nextInt(COLOR_COUNT); |
There was a problem hiding this comment.
| int colorNumber = random.nextInt(COLOR_COUNT); | |
| int colorNumber = random.nextInt(COLORS.length); |
| return null; | ||
| int colorNumber = random.nextInt(COLOR_COUNT); | ||
|
|
||
| return Color.values()[colorNumber].name(); |
There was a problem hiding this comment.
| return Color.values()[colorNumber].name(); | |
| return COLORS[colorNumber].name(); |
|
|
||
| public Ball getRandomBall() { | ||
| String color = colorSupplier.getRandomColor(); | ||
| int number = random.nextInt(100); |
There was a problem hiding this comment.
100 is a magic number, create constant
…ments class ColorSupplier; implements main() in Application class