completed tasks #1555

xli1iax · 2024-07-30T22:55:15Z

No description provided.

mateacademy-ai-mentor

Great job on defining the Color enum according to Java conventions! 🎉 While there are some suggestions for improvements, none of them are critical issues that would require a change request. Keep up the good work, and consider the feedback to enhance your code for the future. 👍 Remember, using static fields for shared resources and creating instances for non-static methods can help make your code more efficient and object-oriented. Happy coding!

mateacademy-ai-mentor · 2024-07-30T22:55:39Z

src/main/java/ball/characteristics/Ball.java

+
+public class Ball {
+    private int number;
+    private String color;


The color field should be of the enum type Color, not String, to enforce the use of predefined color values.

mateacademy-ai-mentor · 2024-07-30T22:55:39Z

src/main/java/ball/characteristics/Ball.java

+        return number;
+    }
+
+    @Override


The toString() method should return a string that represents the entire Ball object, not just the color. Consider including both the number and the color in the returned string.

mateacademy-ai-mentor · 2024-07-30T22:55:40Z

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

+        for (int i = 0; i < 3; i++) {
+            System.out.println(Lottery.getRandomBall());
+        }


Don't use static methods in your solution. Instead of calling a static method on the Lottery class, you should create an instance of the Lottery class and call the method on that instance.

mateacademy-ai-mentor · 2024-07-30T22:55:40Z

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


 public class Application {
    public static void main(String[] args) {
        // create three balls using class Lottery and print information about them in console


Please don't add redundant empty lines to your code. There's no need for an empty line after the main method signature.

mateacademy-ai-mentor · 2024-07-30T22:55:40Z

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

+    public static String getRandomColor() {
+        int index = new Random().nextInt(Color.values().length);
+        Ball ball = new Ball();
+        ball.setColor(Color.values()[index].toString());
+        return ball.getColor();
    }


Avoid using static methods as they are generally considered bad practice in object-oriented programming. You should create an instance of ColorSupplier in the class where you want to use it.

mateacademy-ai-mentor · 2024-07-30T22:55:40Z

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

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


It's unnecessary to create a new instance of Ball in this method. You can directly return the color as a string using Color.values()[index].name().

good advice

mateacademy-ai-mentor · 2024-07-30T22:55:40Z

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

+        ball.setColor(Color.values()[index].toString());
+        return ball.getColor();


Instead of using toString(), use name() for getting the String representation of enum constants to avoid issues if toString() is overridden in the enum.

mateacademy-ai-mentor · 2024-07-30T22:55:40Z

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

+public class Lottery {
+    public static String getRandomBall() {
+        Ball ball = new Ball();
+        ball.setColor(ColorSupplier.getRandomColor());
+        ball.setNumber(new Random().nextInt(100));
+        return "Ball color is " + ball.getColor() + " and the number is " + ball.getNumber();
+    }


Don't use static methods in your solution. Instead, create an instance of the class whose method you want to call. This will make your code more flexible and object-oriented.

mateacademy-ai-mentor · 2024-07-30T22:55:40Z

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

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


The method name getRandomBall suggests that it should return a Ball object, but it currently returns a String. Consider changing the return type to Ball and moving the string representation to a toString method in the Ball class.

mateacademy-ai-mentor · 2024-07-30T22:55:40Z

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

+    public static String getRandomBall() {
+        Ball ball = new Ball();
+        ball.setColor(ColorSupplier.getRandomColor());
+        ball.setNumber(new Random().nextInt(100));


Instead of creating a new instance of Random every time the method is called, consider defining it as a private static final field in the class. This will be more efficient as you will reuse the same Random instance.

MentorMateAcademy · 2024-07-31T12:51:50Z

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

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


MentorMateAcademy · 2024-07-31T12:51:59Z

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

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


Suggested change

public static String getRandomColor() {

public String getRandomColor() {

not fixed - we should avoid static methods, this will become evident in Spring section

MentorMateAcademy · 2024-07-31T12:52:16Z

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

+        ball.setColor(Color.values()[index].toString());
+        return ball.getColor();


MentorMateAcademy · 2024-07-31T12:52:37Z

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

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


good advice

MentorMateAcademy · 2024-07-31T12:53:02Z

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

+        ball.setColor(ColorSupplier.getRandomColor());
+        ball.setNumber(new Random().nextInt(100));


try to create ball using constructor

MentorMateAcademy · 2024-07-31T12:53:11Z

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

+    public static String getRandomBall() {
+        Ball ball = new Ball();
+        ball.setColor(ColorSupplier.getRandomColor());
+        ball.setNumber(new Random().nextInt(100));


MentorMateAcademy · 2024-07-31T12:53:25Z

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

+public class Lottery {
+    public static String getRandomBall() {
+        Ball ball = new Ball();
+        ball.setColor(ColorSupplier.getRandomColor());
+        ball.setNumber(new Random().nextInt(100));
+        return "Ball color is " + ball.getColor() + " and the number is " + ball.getNumber();
+    }


okuzan · 2024-08-01T14:52:49Z

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

+    private final int randomNum = new Random().nextInt(100);
+    private final ColorSupplier colorSupplier = new ColorSupplier();
+    private final Ball ball = new Ball(randomNum, colorSupplier.getRandomColor());
+
+    public Ball getRandomBall() {
+        return ball;
+    }
+}


100 should be a constant
Random should be a private field
random num should a local variable be generated in getRandomBall method. Same for Ball. Right now you only create one unique ball per each lottery instance - it doesn't fulfil our requirements, we want lottery to generate a new Ball every time the method is called

okuzan · 2024-08-01T14:53:03Z

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

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


same for this Random, it should be a field

okuzan · 2024-08-01T14:53:41Z

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

+        for (int i = 0; i < 3; i++) {
+            Lottery lottery = new Lottery();
+            Ball ball = new Ball(lottery.getRandomBall());
+            System.out.println(ball.toString());


you can remove .toString() from println, it's called under the hood anyway

yes, but it make up a sentence at once, is it bad?
I thought that I should use .toString() somewhere, or I just need to override it?

Sorry I didn't really get your question :) You should override toString in the Ball.java class, but when you put something in system out println, .toString of this object will be called

okuzan · 2024-08-02T16:33:24Z

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

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


why public?
BORDER - user uppercase for constants

okuzan · 2024-08-02T16:35:45Z

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

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


not fixed - we should avoid static methods, this will become evident in Spring section

…in class

okuzan

put ball package inside of core/basesyntax, everything should be under one root package

okuzan · 2024-08-03T11:56:03Z

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

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


Suggested change

static final int BORDER = 100;

private static final int BORDER = 100;

okuzan · 2024-08-03T11:56:23Z

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

    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++) {
+            Lottery lottery = new Lottery();


Don't create new lottery instance in a loop - do it outside of the loop

…eated in a loop

Elena-Bruyako · 2024-08-04T13:30:13Z

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

        // create three balls using class Lottery and print information about them in console
+        Lottery lottery = new Lottery();
+
+        for (int i = 0; i < 3; i++) {


3 is a magic number, create constant

Elena-Bruyako · 2024-08-04T13:30:24Z

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

+            Ball ball = new Ball(lottery.getRandomBall());
+            System.out.println(ball);


Suggested change

Ball ball = new Ball(lottery.getRandomBall());

System.out.println(ball);

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

Elena-Bruyako · 2024-08-04T13:31:05Z

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

+        this.number = number;
+    }
+
+    public Ball(Ball ball) {


Elena-Bruyako · 2024-08-04T14:49:01Z

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

    public static void main(String[] args) {
        // create three balls using class Lottery and print information about them in console
+        Lottery lottery = new Lottery();
+        int amountBalls = 3;


this is not constant
make it constant

completed tasks

a4fe25a

mateacademy-ai-mentor approved these changes Jul 30, 2024

View reviewed changes

MentorMateAcademy suggested changes Jul 31, 2024

View reviewed changes

corrected by recommendations

e38af7d

xli1iax requested review from MentorMateAcademy and mateacademy-ai-mentor July 31, 2024 17:04

okuzan suggested changes Aug 1, 2024

View reviewed changes

corrected fields and methods in Lottery.java and ColorSuplier.java

584a6f9

xli1iax requested a review from okuzan August 1, 2024 15:40

okuzan suggested changes Aug 2, 2024

View reviewed changes

fixed getRandomColor method to non-static and removed .toString in ma…

43ff6bf

…in class

xli1iax requested a review from okuzan August 2, 2024 18:25

okuzan suggested changes Aug 3, 2024

View reviewed changes

all fiels are moved into one package and the lottery object is not cr…

13df86d

…eated in a loop

xli1iax requested a review from okuzan August 3, 2024 15:02

Elena-Bruyako suggested changes Aug 4, 2024

View reviewed changes

removed one constructor, made 3 a constant number

1475cf1

xli1iax requested a review from Elena-Bruyako August 4, 2024 14:06

Elena-Bruyako suggested changes Aug 4, 2024

View reviewed changes

made 3 a constant number on class level

c6a01a0

xli1iax requested a review from Elena-Bruyako August 4, 2024 15:06

Elena-Bruyako approved these changes Aug 5, 2024

View reviewed changes

		ball.setColor(Color.values()[index].toString());
		return ball.getColor();

	public static String getRandomColor() {
	public String getRandomColor() {

		ball.setColor(ColorSupplier.getRandomColor());
		ball.setNumber(new Random().nextInt(100));

	static final int BORDER = 100;
	private static final int BORDER = 100;

		Ball ball = new Ball(lottery.getRandomBall());
		System.out.println(ball);

	Ball ball = new Ball(lottery.getRandomBall());
	System.out.println(ball);
	System.out.println(lottery.getRandomBall()));

completed tasks #1555

Are you sure you want to change the base?

completed tasks #1555

Uh oh!

Conversation

xli1iax commented Jul 30, 2024

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xli1iax Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

okuzan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

xli1iax Aug 1, 2024 •

edited

Loading