-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add files via upload #1696
base: master
Are you sure you want to change the base?
Add files via upload #1696
Conversation
There was a problem hiding this 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! 🎉 Your code has been reviewed and is approved to help you move forward with your learning journey. However, there are some important points to address:
-
Ball
Class:- The
toString()
method should return a string representation of the ball that includes both the actual color and the number. Replace'color'
with the actual color value from theColors
enum.
- The
-
Naming Conventions:
- Ensure variable names follow Java naming conventions. For instance, variables like
Ball
,Ball1
,Ball2
,Ball3
, andColorsSupplier
should be renamed toball
,ball1
,ball2
,ball3
, andcolorSupplier
respectively, using camelCase.
- Ensure variable names follow Java naming conventions. For instance, variables like
-
Lottery
Class:- Avoid using static methods like
getRandomBall()
unless necessary. Consider creating an instance of the class instead. - Ensure variable names like
numb
andBall
start with a lowercase letter.
- Avoid using static methods like
-
ColorSupplier
Class:- Optimize the
getRandomColor()
method by creating a singleRandom
instance at the class level instead of creating a new one each time the method is called.
- Optimize the
-
Colors
Enum:- Enum values should be in uppercase to follow Java conventions for constants.
These improvements will enhance the readability and efficiency of your code. Keep up the great work, and continue to learn from these suggestions to refine your coding skills! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
Ball.java
Outdated
public String toString(){ | ||
return "Ball" + "color" + number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The toString
method should return a string that includes the actual color of the ball. Replace 'color'
with color
to use the Colors
enum value, and consider adding a space or other separator between 'Ball', the color, and the number for better readability.
Lottery.java
Outdated
public static Ball getRandomBall(){ | ||
Random random = new Random(); | ||
int numb = random.nextInt(100); | ||
ColorSupplier ColorsSupplier = new ColorSupplier(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable name ColorsSupplier
should follow camelCase naming conventions. Consider renaming it to colorSupplier
.
Lottery.java
Outdated
int numb = random.nextInt(100); | ||
ColorSupplier ColorsSupplier = new ColorSupplier(); | ||
Colors color = ColorsSupplier.getRandomColor(); | ||
Ball Ball = new Ball(numb, color); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable name Ball
should follow camelCase naming conventions. Consider renaming it to ball
.
Main.java
Outdated
// click the <icon src="AllIcons.Actions.Execute"/> icon in the gutter. | ||
public class Main { | ||
public static void main(String[] args) { | ||
Ball Ball1 = Lottery.getRandomBall(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable name Ball1
should follow camelCase naming conventions. Consider renaming it to ball1
.
Main.java
Outdated
public class Main { | ||
public static void main(String[] args) { | ||
Ball Ball1 = Lottery.getRandomBall(); | ||
Ball Ball2 = Lottery.getRandomBall(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable name Ball2
should follow camelCase naming conventions. Consider renaming it to ball2
.
Main.java
Outdated
public static void main(String[] args) { | ||
Ball Ball1 = Lottery.getRandomBall(); | ||
Ball Ball2 = Lottery.getRandomBall(); | ||
Ball Ball3 = Lottery.getRandomBall(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable name Ball3
should follow camelCase naming conventions. Consider renaming it to ball3
.
There was a problem hiding this 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 mostly well-implemented, there are a few areas that need improvement to fully meet the task requirements:
-
Ball Class:
- The
toString()
method is not correctly implemented. It should return a string that includes both the color and number of the ball. Consider using:return "Ball: " + this.color + " " + this.number;
.
- The
-
Lottery Class:
- Avoid using static methods like
getRandomBall()
. It's recommended to instantiate objects instead. - Ensure variable names follow Java naming conventions. For instance,
ColorsSupplier
should becolorSupplier
.
- Avoid using static methods like
-
ColorSupplier Class:
- Instead of creating a new
Random
object each timegetRandomColor()
is called, consider creating a singleRandom
instance at the class level for efficiency.
- Instead of creating a new
-
Main Class:
- The
Ball
instances should not be created using static methods. Adjust this to align with best practices. - Ensure variable names like
Ball1
,Ball2
, andBall3
start with a lowercase letter, e.g.,ball1
,ball2
,ball3
.
- The
-
General Issues:
- Make sure all classes, fields, constructors, and methods have explicit access modifiers.
- Enum values in
Colors
should be in uppercase to follow Java naming conventions for constants.
Despite these issues, your effort is commendable, and the decision to approve is made to help you progress in your learning journey. Please review the comments and address these points in future submissions to enhance your coding skills. Keep up the good work and continue improving! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
Ball.java
Outdated
public String toString(){ | ||
return "Ball" + "color" + number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The toString
method does not correctly concatenate the color
and number
fields. It currently returns the string 'Ballcolor' followed by the number. You should use this.color
and this.number
to properly include the object's fields in the returned string. Consider using: return "Ball: " + this.color + " " + this.number;
Lottery.java
Outdated
public static Ball getRandomBall(){ | ||
Random random = new Random(); | ||
int numb = random.nextInt(100); | ||
ColorSupplier ColorsSupplier = new ColorSupplier(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable name ColorsSupplier
should follow camelCase naming conventions. Consider renaming it to colorSupplier
for consistency with Java naming standards.
Lottery.java
Outdated
int numb = random.nextInt(100); | ||
ColorSupplier ColorsSupplier = new ColorSupplier(); | ||
Colors color = ColorsSupplier.getRandomColor(); | ||
Ball Ball = new Ball(numb, color); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable name Ball
should start with a lowercase letter to follow Java naming conventions. Consider renaming it to ball
.
Main.java
Outdated
Ball Ball1 = Lottery.getRandomBall(); | ||
Ball Ball2 = Lottery.getRandomBall(); | ||
Ball Ball3 = Lottery.getRandomBall(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable names Ball1
, Ball2
, and Ball3
should start with a lowercase letter to follow Java naming conventions. Consider renaming them to ball1
, ball2
, and ball3
.
No description provided.