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

added solution #1281

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

Conversation

Julianevinska
Copy link

No description provided.

Copy link

@Rommelua Rommelua left a comment

Choose a reason for hiding this comment

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

Before you create PR with your homework, you need to go through the checklist under the task and correct all the points described there. The mentor will not check the work until the checklist points are corrected

Comment on lines 6 to 8
Lottery ballOne = new Lottery();
Lottery ballTwo = new Lottery();
Lottery ballThree = new Lottery();

Choose a reason for hiding this comment

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

Lottery is lottery, Ball is ball. We need 1 Lottery and 3 Balls

Comment on lines 3 to 5
public class Ball {

private String color;

Choose a reason for hiding this comment

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

image

@@ -0,0 +1,10 @@
package core.basesyntax;

public enum Colors {

Choose a reason for hiding this comment

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

Suggested change
public enum Colors {
public enum Color {

public class ColorSupplier {
public String getRandomColor() {
return null;
Random random = new Random();

Choose a reason for hiding this comment

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

image

Copy link

@George-Khomiak George-Khomiak left a comment

Choose a reason for hiding this comment

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

Generally good work, but please read checklist carefully

package core.basesyntax;

public class Ball {
private String color;

Choose a reason for hiding this comment

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

Use Enum Color that you created instead of regular String


public class Lottery {

Ball getRandomBall() {

Choose a reason for hiding this comment

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

Add access modifier for this method

Comment on lines 7 to 9
System.out.println(lottery.getRandomBall().toString());
System.out.println(lottery.getRandomBall().toString());
System.out.println(lottery.getRandomBall().toString());

Choose a reason for hiding this comment

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

Use for loop for the repeatable actions. It's the same logic as you have in checklist for this task - "Use for loop for creating several objects of the same class"
image

public class Lottery {

Ball getRandomBall() {
Ball ball = new Ball();

Choose a reason for hiding this comment

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

Move Ball, Color and Random instances creation out of this method - create them at the class level, so they can be reusable. Don't foerget access modifier for these fields once they are at the class level

private Ball ball = new Ball();

public Ball getRandomBall() {
...
}

ball.setColor(color);

Random random = new Random();
int ballIndex = random.nextInt(101);

Choose a reason for hiding this comment

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

image

- Optimized Lottery
- Added loops in Application
- Added access modifiers
Copy link

@George-Khomiak George-Khomiak left a comment

Choose a reason for hiding this comment

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

Great!

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.

3 participants