done #1735

xMaXD · 2024-12-31T13:34:00Z

No description provided.

mateacademy-ai-mentor

Thank you for your hard work on this project! 😊 While you've made a great start, there are a few critical issues that need to be addressed to meet the core requirements of the task:

Ball Class: The toString() method should be overridden to provide a meaningful string representation of the Ball object, including its color and number. This is essential for understanding the output of your program.
ColorSupplier Class: The getRandomColor() method currently modifies the Ball object directly, which is not aligned with the task requirements. It should return a random color instead, allowing for better separation of concerns.
Lottery Class: The getRandomBall() method should return a Ball object, not a String. This is crucial for the functionality of your application.
Application Class: You should use a loop to create multiple Ball objects using the getRandomBall() method from the Lottery class, rather than creating Lottery objects manually. This will improve code maintainability and adhere to the checklist.
Enum Naming: Ensure that the Colors enum follows naming conventions, using uppercase letters for constants.

Please address these issues to ensure your code meets the task requirements. Keep up the great work, and I look forward to seeing your improvements! 🌟

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

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

mateacademy-ai-mentor · 2024-12-31T13:35:36Z

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

@@ -3,5 +3,9 @@
 public class Application {
    public static void main(String[] args) {
        // create three balls using class Lottery and print information about them in console
+        Lottery[] balls = { new Lottery(), new Lottery(), new Lottery() };


Instead of manually creating each Lottery instance, consider using a loop to create the instances. This approach is recommended in the checklist to avoid redundancy and improve code maintainability .

mateacademy-ai-mentor · 2024-12-31T13:35:36Z

src/main/java/core/basesyntax/Ball.java

+    public String toString() {
+        return super.toString();


The toString() method should be overridden to provide a meaningful string representation of the Ball object. Consider returning a string that includes both the color and number of the ball, for example: return "Ball{color=" + color + ", number=" + number + "}";.

mateacademy-ai-mentor · 2024-12-31T13:35:37Z

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

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


The method getRandomColor both assigns a random color to the Ball object and returns the color. This dual responsibility might be unexpected based on the method name. Consider renaming the method to reflect its behavior more accurately, or separate the color generation and assignment into different methods.

mateacademy-ai-mentor · 2024-12-31T13:35:37Z

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

+    public String getRandomBall() {
+        Ball ball = new Ball();
+        ColorSupplier colorSupplier = new ColorSupplier();
+        colorSupplier.getRandomColor(ball);


The getRandomColor method in ColorSupplier modifies the Ball object directly. It might be better to have getRandomColor return a color, which can then be set on the Ball object here. This would improve the separation of concerns and make the code more intuitive.

xMaXD

changed it

mateacademy-ai-mentor

Great job on implementing most of the task requirements! 🎉 Here are some key points to consider for improvement:

Application Class: Instead of creating multiple Lottery objects, you should create a single Lottery object and use it to generate multiple Ball objects using a loop. This will align with the task requirement to create multiple Ball objects.
Ball Class: Well done on overriding the toString() method to provide a meaningful string representation of the Ball object, including its color and number.
ColorSupplier Class: The getRandomColor() method is correctly implemented, ensuring proper separation of concerns.
Lottery Class: The getRandomBall() method is correctly implemented, returning a Ball object with a random color and number.
Enum Naming: The Colors enum follows the naming conventions correctly.

The decision to approve is to help you move forward with your learning. Please review the comments to improve your code. Addressing these issues will enhance your understanding and help avoid similar problems in the future. Keep up the good work and continue learning from your experiences! 🚀

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

mateacademy-ai-mentor · 2024-12-31T13:59:53Z

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

@@ -3,5 +3,9 @@
 public class Application {
    public static void main(String[] args) {
        // create three balls using class Lottery and print information about them in console
+        Lottery[] balls = { new Lottery(), new Lottery(), new Lottery() };


Instead of creating multiple Lottery objects, you should create a single Lottery object and use it to generate multiple Ball objects using the getRandomBall() method. This will align with the task requirement to create multiple Ball objects.

xMaXD

made it with for loop

liliia-ponomarenko

Good job! Let’s improve your solution ;)

liliia-ponomarenko · 2025-01-02T07:44:13Z

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

@@ -3,5 +3,10 @@
 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++) {


Make 3 constant field

liliia-ponomarenko · 2025-01-02T07:44:56Z

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

-    public String getRandomColor() {
-        return null;
+    public Colors getRandomColor() {
+        Random random = new Random();


random should be a private final class level variable

liliia-ponomarenko · 2025-01-02T07:45:03Z

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 {

liliia-ponomarenko · 2025-01-02T07:45:53Z

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

+import java.util.Random;
+
+public class Lottery {
+    public Ball getRandomBall() {


Random and ColorSuplier should be a private final class level variable

liliia-ponomarenko · 2025-01-02T07:46:08Z

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

+public class Lottery {
+    public Ball getRandomBall() {
+        Ball ball = new Ball();
+        ColorSupplier colorSupplier = new ColorSupplier(); // Create a ColorSupplier object


remove redundant comment

liliia-ponomarenko · 2025-01-02T07:46:48Z

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

+        ColorSupplier colorSupplier = new ColorSupplier(); // Create a ColorSupplier object
+        Colors randomColor = colorSupplier.getRandomColor();
+        ball.setColor(randomColor);
+        int randomValue = new Random().nextInt(100);


make magic number constant field

liliia-ponomarenko · 2025-01-02T07:51:11Z

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

+
+public class Lottery {
+    public Ball getRandomBall() {
+        Ball ball = new Ball();


also, you can create a constructor with parameters for the Ball model and simplify your solution.

done

cf5f58c

mateacademy-ai-mentor suggested changes Dec 31, 2024

View reviewed changes

new solution

aa53b99

xMaXD requested a review from mateacademy-ai-mentor December 31, 2024 13:53

xMaXD commented Dec 31, 2024

View reviewed changes

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

View reviewed changes

solution 3

4275095

xMaXD commented Dec 31, 2024

View reviewed changes

liliia-ponomarenko suggested changes Jan 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

done #1735

done #1735

xMaXD commented Dec 31, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Dec 31, 2024

mateacademy-ai-mentor Dec 31, 2024

mateacademy-ai-mentor Dec 31, 2024

mateacademy-ai-mentor Dec 31, 2024

xMaXD left a comment

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Dec 31, 2024

xMaXD left a comment

liliia-ponomarenko left a comment

liliia-ponomarenko Jan 2, 2025

liliia-ponomarenko Jan 2, 2025

liliia-ponomarenko Jan 2, 2025

liliia-ponomarenko Jan 2, 2025

liliia-ponomarenko Jan 2, 2025

liliia-ponomarenko Jan 2, 2025

liliia-ponomarenko Jan 2, 2025

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

		public enum Colors {

done #1735

Are you sure you want to change the base?

done #1735

Conversation

xMaXD commented Dec 31, 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

xMaXD left a comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xMaXD left a comment

Choose a reason for hiding this comment

liliia-ponomarenko 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