Final solution. #1713
Final solution. #1713
Conversation
There was a problem hiding this comment.
Your code is almost there, but there are a couple of critical issues that need to be addressed before we can approve it. 🚀 The main concern is the misuse of inheritance in the Lottery class. Instead of extending ColorSupplier, you should use composition by creating an instance of ColorSupplier within Lottery. This change will align your code with best practices and improve its design. Additionally, consider using a loop to create multiple Ball objects to reduce redundancy. Once these changes are made, your code will be in great shape! Keep up the good 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.
| String color; | ||
| int number; |
There was a problem hiding this comment.
Add access modifiers for each class field
| public class ColorSupplier { | ||
| public String getRandomColor() { | ||
| return null; | ||
| public static String getRandomColor() { |
There was a problem hiding this comment.
From checklist: "Don't use static methods in your solution"
|
|
||
| return randomBall; | ||
| } | ||
| } |
There was a problem hiding this comment.
Add empty line at the end of each files, these are not redundant
| @@ -3,5 +3,10 @@ | |||
| public class Application { | |||
| public static void main(String[] args) { | |||
| // create three balls using class Lottery and print information about them in console | |||
| Ball[] ballArray = new Ball[3]; | |||
| for (int i = 0; i < 3; i++) { | |||
| ballArray[i] = Lottery.getRandomBall(); | |||
There was a problem hiding this comment.
When you remove "static" keyword from Lottery method "getRandomBall" you will need to create an instance of a Lottery class to use it's methods, i.e.
Lottery lottery = new Lottery();
| package core.basesyntax; | ||
|
|
||
| public class Ball { | ||
| String color; |
There was a problem hiding this comment.
Add getters and setters for each class field. If you wish add a constructor to create instances of this class more easily
| public class ColorSupplier { | ||
| public String getRandomColor() { | ||
| return null; | ||
| public static String getRandomColor() { |
There was a problem hiding this comment.
Don't convert Color to a String everywhere, just use your ENUM Color instead.
| package core.basesyntax; | ||
|
|
||
| public class Ball { | ||
| String color; |
There was a problem hiding this comment.
Use Color ENUM here, not a String. So this field should be of type Color, not of a type String
| BLACK, | ||
| GOLD, | ||
| SILVER | ||
| } |
There was a problem hiding this comment.
Perfect, just add a new line at the end of file :) this is our convention
|
Hi. Thank you for checking my code. |
Hi, you did well, let me explain what was off. In the class Ball you had no access modifiers for class fields - color and number - so it was possible to set these values creating a Ball class instance. But this is not the way we write code in Java, we must use access modifiers for classes, variables (class fields in this case) and functions (methods in this case). Please take a look at this topic again if you already had a lecture on it. Also ask quesions in the chat or on QA sessions. |
There was a problem hiding this comment.
Great job generally, now let me explain what was off. In the class Ball you had no access modifiers for class fields - color and number - so it was possible to set these values creating a Ball class instance. But this is not the way we write code in Java, we must use access modifiers for classes, variables (class fields in this case) and functions (methods in this case). Please take a look at this topic again if you already had a lecture on it. Also ask quesions in the chat or on QA sessions.
So, when your class design is good - having access modifiers - if they are private, which is usually the case, you will not have possibility to set values for color and number without Setters. Try it. That's, mostly, the purpose of Getters/Setters.
No description provided.