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.

Sign up for GitHub

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

Jump to bottom

Homework #1247

Open

Kinereiko wants to merge 9 commits into mate-academy:master from Kinereiko:homework

Kinereiko commented Nov 15, 2023

No description provided.

Kinereiko added 2 commits

November 15, 2023 23:36


          Create simple lottery

bd8c521


          Create simple lottery

e10f8aa

Rommelua suggested changes

View reviewed changes

Rommelua left a comment

Before you create PR with your homework, you need to go through the checklist under the task and correct all the points described there. The mentor will not check the work until the checklist points are corrected

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

@@ @@ -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 lottery = new Lottery();
+                      for (int i = 0; i < 3; i++) {
+                          System.out.println(lottery.getRandomBall().toString());

Rommelua Nov 16, 2023

Suggested change

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

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

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

Rommelua Nov 16, 2023

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

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

		public enum Colors {

Rommelua Nov 16, 2023

Suggested change

      
            public enum Colors {
          
            public enum Color {

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

+                  public Ball getRandomBall() {
+                      Ball randBall = new Ball();
+                      randBall.setColor(supplier.getRandomColor());
+                      randBall.setNumber(new Random().nextInt(100));

Rommelua Nov 16, 2023

Kinereiko added 2 commits

November 16, 2023 14:13


          Create simple lottery

a7fef1a


          Corrected previous mistakes

ac9172a

Kinereiko requested a review from Rommelua

November 16, 2023 12:26

sokimaaa suggested changes

View reviewed changes

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

+                  public Ball getRandomBall() {
+                      Ball randBall = new Ball();
+                      randBall.setColor(supplier.getRandomColor());
+                      randBall.setNumber(randNum.nextInt(100));

sokimaaa Nov 16, 2023

100 is a magic number.
Avoid direct use of magic numbers instead you should always make it constant with self-descriptive name.

Suggested change

      
                    randBall.setNumber(randNum.nextInt(100));
          
                    randBall.setNumber(randNum.nextInt(100));

Kinereiko added 2 commits

November 16, 2023 22:23


          Add constant for magic number

af9ba28


          Add constant for magic number

5e85019

Kinereiko requested a review from sokimaaa

November 16, 2023 20:27

Rommelua suggested changes

View reviewed changes

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

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

Rommelua Nov 17, 2023

Suggested change

      
                private static final int maxNumber = 100;
          
                private static final int MAX_BALL_NUMBER = 100;

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

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

Rommelua Nov 17, 2023

Suggested change

      
                private final Random randNum = new Random(100);
          
                private final Random random = new Random();


          Renamed constants

dedb6b2

Kinereiko requested a review from Rommelua

November 17, 2023 12:54

Sanderfuck suggested changes

View reviewed changes

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

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

Sanderfuck Nov 17, 2023

you sent magic number, why if you have constant already?

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

Comment on lines 7 to 21

+                  public Color getColor() {
+                      return color;
+                  }
+                  public void setColor(Color color) {
+                      this.color = color;
+                  }
+                  public int getNumber() {
+                      return number;
+                  }
+                  public void setNumber(int number) {
+                      this.number = number;
+                  }

Sanderfuck Nov 17, 2023

Better use constructor and you don`t to need getters an setters

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

+              package core.basesyntax;
+              public class Ball {
+                  private Color color;

Sanderfuck Nov 17, 2023

Use String for color

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

Comment on lines 8 to 10

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

Sanderfuck Nov 17, 2023

return here String

Kinereiko added 2 commits

November 17, 2023 15:26


          Renamed constants

e7476df


          Add constructor, changed types

Kinereiko requested a review from Sanderfuck

November 17, 2023 13:35

sokimaaa approved these changes

View reviewed changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet