Created program that return balls with a random color and random number #1724

yaroslav-pryshchepa · 2024-12-18T14:37:21Z

No description provided.

mateacademy-ai-mentor

Great job on implementing the task! 🎉 Your code meets the core requirements and functions as expected. While there are a few areas for improvement, such as reusing instances of Lottery and ColorSupplier to enhance efficiency, these are minor and don't impact the overall functionality. Keep up the good work and consider these suggestions for future projects to make your code even more efficient and maintainable! 😊

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

mateacademy-ai-mentor · 2024-12-18T14:38:35Z

src/main/java/core/basesyntax/Application.java

+        Ball ball1 = new Lottery().getRandomBall();
+        Ball ball2 = new Lottery().getRandomBall();
+        Ball ball3 = new Lottery().getRandomBall();


You are creating multiple instances of the Lottery class. According to the checklist, it's better to create a single instance and reuse it. Consider creating one Lottery instance and using it to generate all three balls.

mateacademy-ai-mentor · 2024-12-18T14:38:35Z

src/main/java/core/basesyntax/Lottery.java

+public class Lottery {
+    Ball getRandomBall() {
+        Ball ball = new Ball();
+        ball.color = String.valueOf(new ColorSupplier().getRandomColor());


You are creating a new ColorSupplier instance each time getRandomBall() is called. Consider creating a single instance of ColorSupplier as a class member to avoid unnecessary object creation.

Elena-Bruyako · 2024-12-18T15:01:32Z

src/main/java/core/basesyntax/Colors.java

@@ -0,0 +1,10 @@
+package core.basesyntax;
+
+public enum Colors {


Suggested change

public enum Colors {

public enum Color {

Elena-Bruyako · 2024-12-18T15:01:47Z

src/main/java/core/basesyntax/ColorSupplier.java

-    public String getRandomColor() {
-        return null;
+    public Colors getRandomColor() {
+        int index = new Random().nextInt(Colors.values().length);


Random should be class-level variable

Elena-Bruyako · 2024-12-18T15:01:56Z

src/main/java/core/basesyntax/ColorSupplier.java

 public class ColorSupplier {
-    public String getRandomColor() {
-        return null;
+    public Colors getRandomColor() {


Suggested change

public Colors getRandomColor() {

public String getRandomColor() {

Elena-Bruyako · 2024-12-18T15:02:35Z

src/main/java/core/basesyntax/Lottery.java

+    Ball getRandomBall() {
+        Ball ball = new Ball();
+        ball.setColor(String.valueOf(colorSupplier.getRandomColor()));
+        ball.setNumber(new Random().nextInt(100));


Random should be class-level variable
create constant for magic number 100

Elena-Bruyako · 2024-12-18T15:03:50Z

src/main/java/core/basesyntax/Application.java

@@ -2,6 +2,9 @@

 public class Application {
    public static void main(String[] args) {
-        // create three balls using class Lottery and print information about them in console
+        for (int i = 0; i < 3; i++) {


3 is a magic number, create constant

Elena-Bruyako · 2024-12-18T15:04:13Z

src/main/java/core/basesyntax/Lottery.java

+public class Lottery {
+    private final ColorSupplier colorSupplier = new ColorSupplier();
+
+    Ball getRandomBall() {


Suggested change

Ball getRandomBall() {

public Ball getRandomBall() {

Elena-Bruyako · 2024-12-18T15:04:55Z

src/main/java/core/basesyntax/Application.java

@@ -2,6 +2,9 @@

 public class Application {
    public static void main(String[] args) {
-        // create three balls using class Lottery and print information about them in console
+        for (int i = 0; i < 3; i++) {
+            Ball ball = new Lottery().getRandomBall();


don't create new Lottery in for loop

Elena-Bruyako · 2024-12-18T16:53:21Z

src/main/java/core/basesyntax/ColorSupplier.java

    public String getRandomColor() {
-        return null;
+        int index = random.nextInt(Color.values().length);
+        return String.valueOf(Color.values()[index]);


Use name() for getting String representation of enum constants

Elena-Bruyako · 2024-12-18T16:55:16Z

src/main/java/core/basesyntax/Lottery.java

+import java.util.Random;
+
+public class Lottery {
+    private static final int maxNumber = 100;


Don't forget how to name constants according to style guide (the same with naming of enum values)

Elena-Bruyako · 2024-12-18T16:56:10Z

src/main/java/core/basesyntax/Application.java

+        Ball ball1 = lottery.getRandomBall();
+        Ball ball2 = lottery.getRandomBall();
+        Ball ball3 = lottery.getRandomBall();
+        Ball[] balls = {ball1, ball2, ball3};
+        for (Ball ball: balls) {
+            System.out.println(ball);
+        }


Suggested change

Ball ball1 = lottery.getRandomBall();

Ball ball2 = lottery.getRandomBall();

Ball ball3 = lottery.getRandomBall();

Ball[] balls = {ball1, ball2, ball3};

for (Ball ball: balls) {

System.out.println(ball);

}

Lottery lottery = new Lottery();

for (int i = 0; i < NUMBER_OF_BALLS; i++) {

System.out.println(lottery.getRandomBall());

}

create constant NUMBER_OF_BALLS

Elena-Bruyako · 2024-12-18T16:56:45Z

src/main/java/core/basesyntax/Lottery.java

+
+public class Lottery {
+    private static final int maxNumber = 100;
+    private static final Random random = new Random();


Suggested change

private static final Random random = new Random();

private final Random random = new Random();

okuzan

You're almost there!

okuzan · 2024-12-19T18:16:47Z

src/main/java/core/basesyntax/Lottery.java

+
+public class Lottery {
+    private static final int MAX_NUMBER = 100;
+    private static final Random RANDOM = new Random();


Suggested change

private static final Random RANDOM = new Random();

private final Random random = new Random();

let's not overuse static - you will be using java with Spring Boot, and we don't use static to inject dependencies there
also, beware that final + static doesn't always mean constants (it's debatable, some say object must be immutable too)

okuzan · 2024-12-19T18:17:06Z

src/main/java/core/basesyntax/Lottery.java

+        ball.setColor(colorSupplier.getRandomColor());
+        ball.setNumber(RANDOM.nextInt(MAX_NUMBER));


minor thing - consider creating a constructor for those two fields

…elds

okuzan

😎

Created program that return balls with a random color and random number

c9c03b6

mateacademy-ai-mentor approved these changes Dec 18, 2024

View reviewed changes

added checkstyle fixes and optimize the solution

6360247

Elena-Bruyako suggested changes Dec 18, 2024

View reviewed changes

added fixes

8d2b6d0

yaroslav-pryshchepa requested a review from Elena-Bruyako December 18, 2024 16:41

Elena-Bruyako suggested changes Dec 18, 2024

View reviewed changes

added fixes include style

569884c

yaroslav-pryshchepa requested a review from Elena-Bruyako December 19, 2024 15:21

okuzan suggested changes Dec 19, 2024

View reviewed changes

added fixes and created a constructor for colorSupplier and random fi…

cd28430

…elds

yaroslav-pryshchepa requested a review from okuzan December 19, 2024 18:53

okuzan approved these changes Dec 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Created program that return balls with a random color and random number #1724

Created program that return balls with a random color and random number #1724

yaroslav-pryshchepa commented Dec 18, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Dec 18, 2024

mateacademy-ai-mentor Dec 18, 2024

Elena-Bruyako Dec 18, 2024

Elena-Bruyako Dec 18, 2024

Elena-Bruyako Dec 18, 2024

Elena-Bruyako Dec 18, 2024

Elena-Bruyako Dec 18, 2024

Elena-Bruyako Dec 18, 2024

Elena-Bruyako Dec 18, 2024

Elena-Bruyako Dec 18, 2024

Elena-Bruyako Dec 18, 2024

Elena-Bruyako Dec 18, 2024

Elena-Bruyako Dec 18, 2024

Elena-Bruyako Dec 18, 2024

okuzan left a comment

okuzan Dec 19, 2024

okuzan Dec 19, 2024

okuzan left a comment

		@@ -0,0 +1,10 @@
		package core.basesyntax;

		public enum Colors {

	public Colors getRandomColor() {
	public String getRandomColor() {

	private static final Random random = new Random();
	private final Random random = new Random();

		ball.setColor(colorSupplier.getRandomColor());
		ball.setNumber(RANDOM.nextInt(MAX_NUMBER));

Created program that return balls with a random color and random number #1724

Are you sure you want to change the base?

Created program that return balls with a random color and random number #1724

Conversation

yaroslav-pryshchepa commented Dec 18, 2024

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

okuzan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

okuzan left a comment

Choose a reason for hiding this comment