-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
The task is ready for verification #1715
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on implementing the task requirements! 🎉 The code is well-structured and meets the core requirements. Just a small note: the Application
class creates four balls instead of three, as specified. Consider adjusting this for consistency. Keep up the excellent work! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job overall!
Random random = new Random(); | ||
int randomNumber = random.nextInt(100) + 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make Random a private final class field, 100 - a class constant, don't forget about constant case
|
||
@Override | ||
public String toString() { | ||
return "Колір: " + color + ", номер " + number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only ever use English in code
Lottery lottery = new Lottery(); | ||
Ball ball1 = lottery.getRandomBall(); | ||
Ball ball2 = lottery.getRandomBall(); | ||
Ball ball3 = lottery.getRandomBall(); | ||
|
||
System.out.println(ball1); | ||
System.out.println(ball2); | ||
System.out.println(ball3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not extensible approach ,what if we had to print 100 times? Read common mistakes checklist found at the task description
Color[] colors = Color.values(); | ||
Random random = new Random(); | ||
int randomIndex = random.nextInt(colors.length); | ||
return colors[randomIndex].toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
common mistake, use .name() for enum
public class ColorSupplier { | ||
public String getRandomColor() { | ||
return null; | ||
Color[] colors = Color.values(); | ||
Random random = new Random(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
random - same comment as for random in color supplier
public class ColorSupplier { | ||
public String getRandomColor() { | ||
return null; | ||
Color[] colors = Color.values(); | ||
Random random = new Random(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Random should be class-level variable
Color[] colors = Color.values(); | ||
Random random = new Random(); | ||
int randomIndex = random.nextInt(colors.length); | ||
return colors[randomIndex].toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use name()
instead of toString()
for enum
check common mistakes file for this task
import java.util.Random; | ||
|
||
public class Lottery { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public class Lottery { | ||
|
||
private static final int MAX_NUMBER = 100; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
private final Random random = new Random(); | ||
|
||
public Ball getRandomBall() { | ||
ColorSupplier colorSupplier = new ColorSupplier(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ColorSupplier should be class-level variable
ColorSupplier colorSupplier = new ColorSupplier(); | ||
|
||
String randomColor = colorSupplier.getRandomColor(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove empty lines in method
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(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need create three balls using class Lottery and print information about them in console
let's create constant for number of balls
create Lottery object
use for loop
and System.out.println(lottery.getRandomBall());
No description provided.