patternjavaModerate
Kings Cup drinking game
Viewed 0 times
cupgamedrinkingkings
Problem
I'm working on a card game in Java that simulates the drinking game Kings Cup for a school project. I'm having trouble putting the pieces together, and was wondering if someone could tell me what I could do to improve my code.
```
public class Human extends Player{
Scanner in;
public Human(boolean turn) {
sup
@author :Kimberly
IDE :NETBEANS
public class Card extends Deck {
enum Rank {
ACE ("Ace"),
TWO ("2"),
THREE ("3"),
FOUR ("4"),
FIVE ("5"),
SIX ("6"),
SEVEN ("7"),
EIGHT ("8"),
NINE ("9"),
TEN ("10"),
JACK ("Jack"),
QUEEN ("Queen"),
KING ("King");
String rank;
Rank(String rank) {
this.rank = rank;
}
}
enum Suit {
HEARTS ("Hearts"),
DIAMONDS ("Diamonds"),
SPADES ("Spades"),
CLUBS ("Clubs");
String name;
Suit(String name) {
this.name = name;
}
}
public Rank rank;
public Suit suit;
Card(Suit suit, Rank rank){
this.rank=rank;
this.suit=suit;
}
public @Override String toString(){
return rank.rank + " of " + suit.name;
}
}public class Deck {
private int dealt;
/**
* Shuffles the deck of cards.
*/
public static ArrayList cards;
Deck() {
cards = new ArrayList();
for (Card.Suit suit : Card.Suit.values()){
for (Card.Rank rank : Card.Rank.values()){
cards.add( new Card(suit,rank));
}
}
Collections.shuffle(cards, new Random());
Collections.shuffle(cards, new Random(System.nanoTime()));
}
public Card getCard(){
return cards.get(0);
}
public void removeFromDeck(){
cards.remove(0);
}
}```
public class Human extends Player{
Scanner in;
public Human(boolean turn) {
sup
Solution
I'm probably missing a fair bit, but these are the things I picked up on:
Why does Card extend Deck? They are both objects in their own right and don't require inheritance.
If Player is intended to be an object it shouldn't have any static variables or methods. You might also want to consider making it an abstract class as you don't want to be able to make a new
You seem to be calling Player.sip() five times in a row manually. To make things easier, I would create another sip() method which takes an
Your rhyme words algorithm is entirely wrong. Firstly, you are only checking if the first element in the array of words contains the answer (which it won't, unless they type the same String five times)
I would change RHYME_WORDS to be a HashSet which actually has a
Consider extracting each of the different card types to its own private method, so you don't just have one massive block of code, and it will make testing each one easier.
I'm not entirely convinced your program works how you want it to at the moment. You should usually make sure you get everything working how it should be before you consider submitting it for code review, as we're not here to test it for you we are here to offer advice on improvement.
Hopefully the points I've raised will give you some direction towards improvement, and then you can keep working on it and report back if you need more help.
Why does Card extend Deck? They are both objects in their own right and don't require inheritance.
If Player is intended to be an object it shouldn't have any static variables or methods. You might also want to consider making it an abstract class as you don't want to be able to make a new
Player, you only want to be able to make a new instance of Player such as a Human or a ComputerYou seem to be calling Player.sip() five times in a row manually. To make things easier, I would create another sip() method which takes an
int parameter of how many times you want to do it, i.e:public void sip(int times) {
for (int i = 0; i < times; i++) {
sip();
}
}Your rhyme words algorithm is entirely wrong. Firstly, you are only checking if the first element in the array of words contains the answer (which it won't, unless they type the same String five times)
if(RHYME_WORDS[0].contains(rhymeAnswer)) {I would change RHYME_WORDS to be a HashSet which actually has a
contains method which will do what you are trying to achieve.Consider extracting each of the different card types to its own private method, so you don't just have one massive block of code, and it will make testing each one easier.
I'm not entirely convinced your program works how you want it to at the moment. You should usually make sure you get everything working how it should be before you consider submitting it for code review, as we're not here to test it for you we are here to offer advice on improvement.
Hopefully the points I've raised will give you some direction towards improvement, and then you can keep working on it and report back if you need more help.
Code Snippets
public void sip(int times) {
for (int i = 0; i < times; i++) {
sip();
}
}if(RHYME_WORDS[0].contains(rhymeAnswer)) {Context
StackExchange Code Review Q#47310, answer score: 10
Revisions (0)
No revisions yet.