Skip to content
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

solution lottery #1706

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

solution lottery #1706

wants to merge 4 commits into from

Conversation

Serhiircv
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


Thank you for your submission! While your code is on the right track, there are a few critical issues that need to be addressed to meet the core requirements of the task:

  1. Lottery Class:

    • The Lottery class should not extend ColorSupplier as it does not represent a "is-a" relationship. This inheritance is unnecessary and incorrect.
    • The randomColor and randomInt fields are incorrectly used. They should be generated within the getRandomBall() method to ensure each Ball object has a unique color and number.
  2. ColorSupplier Class:

    • The randomColor field should be removed, and the getRandomColor() method should be used directly to ensure a new random color is generated each time.
  3. Application Class:

    • Creating multiple instances of Lottery is unnecessary. Use a loop to generate multiple random balls with a single instance for better maintainability and readability.
  4. Enum Usage:

    • Use the name() method instead of String.valueOf() to get the string representation of enum constants, as it ensures the correct representation is always returned.

These changes are essential to ensure the functionality and correctness of your code. Keep up the good work, and I'm confident you'll nail it with these adjustments! 😊

✨ 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.

Comment on lines 6 to 8
Lottery ball1 = new Lottery();
Lottery ball2 = new Lottery();
Lottery ball3 = new Lottery();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a loop to create multiple instances of the Lottery class. This approach is recommended for better code maintainability and readability, as mentioned in the checklist: 'Use for loop for creating several objects of the same class'.

public String getRandomColor() {
return null;
int index = new Random().nextInt(Colors.values().length);
return String.valueOf(Colors.values()[index]);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using String.valueOf(Colors.values()[index]), use Colors.values()[index].name() to get the string representation of the enum constant. This follows the checklist recommendation to use name() for getting String representation of enum constants.

Comment on lines 6 to 7
Random random = new Random();
int randomInt = random.nextInt(100);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The randomInt and randomColor are initialized only once and reused for every Ball object. This means all Ball objects will have the same color and number. Move the random number generation and color assignment inside the getRandomBall() method to ensure each Ball has a unique color and number.

Comment on lines 11 to 12
randomBall.setColor(randomColor);
randomBall.setNumber(randomInt);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the random color and number generation inside the getRandomBall() method to ensure each Ball object has a unique color and number. Call getRandomColor() and random.nextInt(100) within the method.

@Serhiircv Serhiircv closed this Nov 28, 2024
@Serhiircv Serhiircv reopened this Nov 28, 2024
@Serhiircv Serhiircv closed this Nov 28, 2024
@Serhiircv Serhiircv reopened this Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants