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

Done #1736

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

Done #1736

wants to merge 2 commits into from

Conversation

Hot00Tea
Copy link

@Hot00Tea Hot00Tea commented Jan 5, 2025

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.


Hello! 😊 It looks like you're almost there, but there are a couple of critical issues that need to be addressed to meet the core requirements of the task:

  1. Ball Class: The Ball class needs a constructor to initialize the color and number fields. This is essential for creating Ball objects correctly.

  2. Lottery Class: The getRandomBall() method should return a Ball object instead of an int. It should create a Ball with a random color and number, and return this object.

These changes are crucial for the functionality of your code. Once you make these adjustments, your solution will be in great shape! Keep up the good work, and don't hesitate to reach out if you need further assistance. 🌟

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

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

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(color), use color.name() to get the string representation of the enum constant. This is a more reliable method as per the checklist guidelines.

Comment on lines +13 to +17
public int getRandomBall() {
System.out.println(colorSupplier.getRandomColor());
int number = random.nextInt(MAX_VALUE);
return number;
}

Choose a reason for hiding this comment

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

The getRandomBall method should return a Ball object instead of an int. You need to create a new Ball instance using the random color and number, and return that instance. This aligns with the task requirement to create and return a ball with both color and number.

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 effort on implementing the Application, ColorSupplier, and Colors classes! 🎉 However, there are a few critical issues that need attention:

  1. Ball Class Constructor: The Ball class currently lacks a constructor to initialize the color and number fields. Adding a constructor that takes these parameters is essential for creating Ball objects with the necessary attributes.

  2. Lottery Class getRandomBall Method: The getRandomBall() method should return a Ball object instead of an int. Ensure that this method creates a Ball with a random color and number, and returns this object. Additionally, the Ball instance should not be a field in the Lottery class; instead, create a new Ball instance within the getRandomBall() method.

These changes are crucial for the functionality of your code. Although the decision is to APPROVE this pull request to help you move forward with your learning, please review the comments and address these issues to improve your code. Keep up the good work and continue learning from these experiences! 🚀

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


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

Choose a reason for hiding this comment

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

The Ball class needs a constructor to initialize the color and number fields. This is crucial for creating Ball objects correctly. Consider adding a constructor that takes color and number as parameters.

Comment on lines +15 to +18
public int getRandomBall() {
System.out.println(colorSupplier.getRandomColor());
int number = random.nextInt(MAX_VALUE);
return number;

Choose a reason for hiding this comment

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

The getRandomBall() method should return a Ball object, not an int. You need to create a new Ball instance using the random color and number, and return this Ball object.


private final Random random = new Random();

private final Ball ball = new Ball();

Choose a reason for hiding this comment

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

The Ball instance should not be a field in the Lottery class. Instead, create a new Ball instance within the getRandomBall() method.

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