trjhtr

First a minor thing about the code style: don't put else catch while... after a on a new line. Put it on the same line as the }instead. For example:

 do 
 System.out.print("Welcome! Please enter your name: ");
 player.setName(input.nextLine());
 while(player.getName().isEmpty());

I noticed the following 2 lines when I was quickly scrolling through your code and thought it was a really nasty bug first:

 }
 while(player.getName().isEmpty());

This is because I didn't immediatly considered that there might be a do block. If the player name was empty and you had just the statement while(true); which would run forever. Putting it on the same line as the } while would immediatly make it clear that the block before it, is part of that while.

Do not provide setters unless if it actually makes sense to change that value.

For example: If I create a player Player me = new Player("Imus", 10); and play the game. Then at the end if I would print out the player, I expect it to still have the name Imus. But since you got a setter for the name field, we can never be sure nobody else has changed my name in the mean time.

Although the coin value does change, you already have an increment and decrement method which we expect to be used. So the setCoins(int coins) is also not needed.

It's far easier to add a setter to a class in case you need it than it is to remove a public method (like a setter) when you don't need it. This is because you may not know which other classes call that method. Especially important on public API's that other people you don't know about may be using.

At least it's a good thing that you got all the fieldsprivate and provided access to them through getters and setters. This is the information hiding that Timothy mentioned in the good coding principles.

Now for your abstract class.

The problem is that you're trying to apply your learned concepts to a really simple example. If you expect a Dice to have a certain number of faces, and be able to roll a number corresponding to one of those faces. Then the roll() method could just as well be implemented in the (abstract) class as well:

public abstract class Dice 
 private int numFaces;

 public Dice(int numFaces) 
 this.numFaces = numFaces;
 

 public int getNumFaces() 
 return numFaces;
 

 public int roll() 
 return ThreadLocalRandom.current().nextInt(1,6+1); 
 

(quick note here: no setter, but a constructor that sets the numFaces fields.)

And since this class actually has meaning if it is instanciated on it's own it probably should not be abstract anymore. The only thing the subclass adds is a way to create a six sided die without passing the 6 into the constructor. This might as well just be a factory method instead:

public static Dice createSixSidedDice() 
 return new Dice(6);

Either put this method into a specialised DiceFactory class for example. Or just place it inside the Dice class.
Added bonus:
If you only ever want that someone who uses this class can create 6-sided and 4-sided Dice, you can also make the constructor private and provide the 2 static factory methods to create exactly those 2 types of Dice.

Interface <-> Abstract classes.

What you described was the academic view of abstract classes. In practice we use an abstract class when we have 2 classes that have some common functionality but that common "thing" in itself cannot be used without one of those subclasses.
The typical example is indeed something like Cats and Dogs. They have some common Pet traits. So they might have a name, an owner, and they're both able to makeASound() which differs based on if it was a cat or a dog that makes that sound. But you can't do new Animal() and expect a meaningful instance in return. So the classes would look like this:

public abstract class Pet 
 private String name;
 private String owner;

 public Pet(String name, String owner)
 this.name = name;
 this.owner = owner;
 

 //getters for name and owner

 public abstract String makeASound();


public class Cat extends Pet 
 public Cat(String name, String owner)
 super(name, owner);
 

 @override
 public String makeASound()
 return "Miauw!";
 


//similar class for Dog but other sound.

When we define an interface we're only making an agreement. For example: If we want to define a Dice as something that you roll() and that then gives back a number without caring how it does that or even how many faces it has, we can define this as the interface:

public interface Dice 
 public int roll();

Now we can define all sorts of Dice as long as they implement that roll() method. For example:

public class FairSixSidedDice implements Dice 
 public int roll() 
 return ThreadLocalRandom.current().nextInt(1,7); 
 

Or an unfair Dice that always rolls a six:

public class FairSixSidedDice implements Dice 
 public int roll() 
 return 6; 
 

Or some non-transitive dice to use in some interesting games :)

Note again, that in such a small artificial example it doesn't really shows too much of why this is the best solution.

If you want a real life example of how an interface is an agreement I suggest you take a look at the java AutoClosable interface. This interface says that it doesn't matter what the class actually is, it can be closed. No matter if it's a Port, a Stream, a Channel ...

And since we know it can be closed, it can be used in the try-with-resources construct:

try(Scanner scanner = new Scanner(someInputStream) {

Which means we can be sure that it will be cleaned up when the try finishes, no matter if it's finished normally, or by an exception.

The Main class is perfectly fine as it is now. You got to start somewhere right?
An alternative would be to remove that class and put the main method insed the GameMenu class instead.

The issue I have with the GameMenu class is that it actually does 2 major things. It handles the main menu of the game where you input the player and choose to play a game or quit.
But it also handles the "roll the same number in 3 dice rolls" game.

This game should probably earn it's own class. And if you add different games they probably should be in their own respective classes. That way, you can just add an option to the main game menu class to play each of the games. And if that option is picked initiate the corresponding game.

This concept is called the SRP (Single Responsibility Principle). Which means that each class should have only 1 main responsibilty (on a certain level of abstraction).

It's hard to tell how much a class is allowed to do so you'll have to get a feel for it on your own. A good rule of thumb is that you should be able to explain what a class does in a sentence or 3. The things you don't mention in those few sentences should follow implicitly. For example:

The GameMenu class shows the user a number of games to play and let's him pick
which one or let's him quit the game if he no longer wants to play.

How to display the menu and user input for which game to play are implicit here. How to play that dice game is not.

Note that if the user input was more complicated this might no longer be implicit. In that case "how user input is handled" could fit in it's own class. Then the GameMenu class will call this UserInput class to see which option the user chooses. And then based on that input will start up the corresponding game or quit the program.

Thanks for sharing your code.

OOP means that you follow certain principles which are (among others):

• information hiding / encapsulation


• single responsibility


• separation of concerns


• KISS (Keep it simple (and) stupid.)


• DRY (Don't repeat yourself.)


• "Tell! Don't ask."


• Law of demeter ("Don't talk to strangers!")

We inherit from a super class if we extend its behavior. This is: we override a method to do something more and/or something different then the same method in the super class.

You classes Dice and SixFacesDice are a bad example.
^{Correct singular is Die by the way...}

The method roll in SixFacesDice uses the "magic number" 6. This magic number happens to be the same you pass as constructor parameter to it's super class. This means you should better use the super classes variable for the calculation. But this violates information hiding and encapsulation principle. Therefore the implementation of method roll() should be in the super class entirely.

But then your SixFacesDice only configures the super class. Consequently SixFacesDice should not exist and Dice should not be abstract.

The only reason you should have a child class of Dice would be if you want to create a "cheating" die that e.g. always returns a certain number or otherwise "modifies" the randomness of results...