added students solution to review#1
Conversation
| Lottery ball1 = new Lottery(); | ||
| System.out.println(ball1.getRandomBall().toString()); | ||
| Lottery ball2 = new Lottery(); | ||
| System.out.println(ball2.getRandomBall().toString()); | ||
| Lottery ball3 = new Lottery(); | ||
| System.out.println(ball3.getRandomBall().toString()); |
There was a problem hiding this comment.
Use for loop for creating several objects and output of the same data
| } | ||
|
|
||
| public String toString() { | ||
| String ball = "ball"; |
There was a problem hiding this comment.
avoid variables you don't use
|
|
||
| public static String getRandomColor() { | ||
| int index = new Random().nextInt(Colors.values().length); | ||
| return Colors.values()[index].toString(); |
There was a problem hiding this comment.
It’s better to use standard method of enum name() that will be returning always String representation of concrete enum constant.
| return null; | ||
|
|
||
|
|
||
| public static String getRandomColor() { |
There was a problem hiding this comment.
Static methods are in general a bad practice. Let’s better create an instance of a class which method you want to call
| String ballColor; | ||
| int ballNumber; |
There was a problem hiding this comment.
use the private access modifier for internal variables
|
|
||
| Ball ball = new Ball(); | ||
|
|
||
| ball.setBallColor(ColorSupplier.getRandomColor()); | ||
|
|
||
| ball.setBallNumber(new Random().nextInt(100)); | ||
|
|
There was a problem hiding this comment.
Please don’t add redundant empty lines to your code.
| public class Lottery extends ColorSupplier { | ||
| public Ball getRandomBall() { | ||
|
|
||
| Ball ball = new Ball(); |
There was a problem hiding this comment.
you can add parameters to the constructor and avoid calling setters
| public class Lottery extends ColorSupplier { | ||
| public Ball getRandomBall() { | ||
|
|
||
| Ball ball = new Ball(); |
There was a problem hiding this comment.
Don’t create temporary variables if you can directly return value from the method
|
|
||
| import java.util.Random; | ||
|
|
||
| public class Lottery extends ColorSupplier { |
There was a problem hiding this comment.
there is no benefit from this "extends"
|
|
||
| ball.setBallColor(ColorSupplier.getRandomColor()); | ||
|
|
||
| ball.setBallNumber(new Random().nextInt(100)); |
There was a problem hiding this comment.
All magic numbers in your code should be constants.
2 participants
No description provided.