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

Homework #1722

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

Homework #1722

wants to merge 3 commits into from

Conversation

serhii-hl
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 hard work on this project! 😊 There are a few critical issues that need to be addressed before we can approve the pull request:

  1. Method Naming: The method returnBall() in the Lottery class should be renamed to getRandomBall() to align with the task requirements.

  2. Random Number Range: Ensure that the random number generated for the ball is between 0 and 100 inclusive. Update the code to use random.nextInt(101).

  3. Testing in Main Method: The main() method should create and print three balls, not just one. This is important to fully test the functionality.

Please make these changes to meet the core requirements of the task. Once updated, your code will be in great shape! Keep up the great work! 🚀

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

src/main/java/core/basesyntax/ColorSupplier.java Outdated Show resolved Hide resolved
@serhii-hl
Copy link
Author

Done, please review

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.

Review Summary

Great job on implementing the core functionality of the Lottery application! 🎉 However, there are a few critical issues that need your attention to fully meet the task requirements:

  1. Method Naming: The method returnBall() in the Lottery class should be renamed to getRandomBall() to align with the task requirements. This will ensure consistency and clarity in your code.

  2. Random Number Range: The random number generation for the ball should include 100 as a possible value. Currently, the implementation uses random.nextInt(100), which generates numbers from 0 to 99. Please update it to random.nextInt(101) to cover the full range from 0 to 100 inclusive.

  3. Testing in Main Method: The main() method in the Application class should create and print three balls, not just one. This is crucial to fully test the functionality and ensure that your application behaves as expected.

These changes are important for the functionality and correctness of your code. Please review the comments and make the necessary adjustments. Remember, addressing these issues will not only improve your code but also enhance your learning experience. Keep up the good work and continue to learn from these opportunities! 🚀

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

@@ -2,6 +2,7 @@

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();
System.out.println(lottery.returnBall());

Choose a reason for hiding this comment

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

The method returnBall() should be renamed to getRandomBall() to align with the task requirements. Also, ensure that the main method creates and prints three balls, not just one.

import java.util.Random;

public class Lottery {
public Ball returnBall() {

Choose a reason for hiding this comment

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

The method returnBall() should be renamed to getRandomBall() to align with the task requirements.

ColorSupplier colorSupplier = new ColorSupplier();
randomBall.setColor(colorSupplier.getRandomColor());
Random random = new Random();
randomBall.setNumber(random.nextInt(100));

Choose a reason for hiding this comment

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

The random number generation should include 100 as a possible value. Use random.nextInt(101) to ensure the range is 0 to 100 inclusive.

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