3
\$\begingroup\$

This request is a new version of this request: Kings Drinking Game

This version has a custom JForm GUI and almost all new methods.

GUI: Kings GUI

I wrote a program to simulate the drinking card game Kings, now with a JFrom GUI. The GUI is also now not hard-coded into the business code and any UI can be easily used with little adaptation.

I would like a review of my code and some pointers on making it more efficient, cleaner, easier to read, or any other general pointers you have.

The explanation of the game is in the code comment box here:

/**
 * @author              :KyleMHB
 * Project Number       :0003 V2.0
 * Project Name         :Kings
 * Project Path         :Kings/SourcePackages/JFrameKings/Kings.java
 * IDE                  :NETBEANS
 * Goal of Project      - 
 * MainFile is a rule based drinking game using cards for 4+ players.
 * The Rules are read in from a rules.txt so that one can easily change the rules.
 * How the game works:
 * Players shuffle a deck of cards, place a glass between them and circle the
 * cards around the base of the glass.
 * The players then take turns picking cards, each card has its own rule associated to it.
 * Most importantly, there are 4 MainFile, each time a King is picked, 
 * the player who picked it can pour as much of his/her drink into the glass between
 * them as they wish.
 * The game ends when the fourth and final King is picked.
 * The player to pick the final King must down the glass in the center of table.
 */

Imports:

package JFrameKings;

import GUI.KingsGameGUI;
import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Random;
import javax.swing.JOptionPane;

enum declaration for Rank:

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 name;
    Rank(String name) {
        this.name = name;
    }
} 

enum declaration for Suit:

enum Suit {
    HEARTS      ("Hearts"),
    DIAMONDS    ("Diamonds"),
    SPADES      ("Spades"),
    CLUBS       ("Clubs");
    String name;
    Suit(String name) {
        this.name = name;
    }
}

Card Class:

public static class Card {
    public Rank rank;
    public Suit suit;

    Card(Suit suit, Rank rank){
            this.rank=rank;
            this.suit=suit;
    }
    public @Override String toString(){
              return rank.name + " of " + suit.name;
    }
}

Deck Class

public static class Deck {
    public static ArrayList<Card> cards;
     Deck() {
         cards=new ArrayList<Card>();
            for (Suit suit : Suit.values()){
                    for (Rank rank : 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);
     }
}

PSVM() and static declarations:

public static List<String> rules;
public static int playerTurn=1;

public static final int players=getNum("How many people are going to play",
                                       "Number of Players");
private static int kings=0;
private static Deck deck=new Deck();

public static void main(String[] args) throws IOException{     
    setRules("rules.txt");
    KingsGameGUI.main(null);
}

getNum() Method:

private static int getNum(String prompt,String title) {
    return Integer.parseInt
            (JOptionPane.showInputDialog(null,prompt,title,3));
}

setRules() Method:

/**
* Rules are not hard-coded because people often have different ones,
* Therefore I made an easily editable rules.txt file.
* Also my rule file has formatting in using the \n,
* However when the file is read it is read as \ and n
* Hence why I used the replaceAll( "\\\\n","\n");
*/
private static void setRules(File f) throws FileNotFoundException, IOException {
    rules = Files.readAllLines(f.toPath(), Charset.defaultCharset());
    for(int i=0; i!=rules.size(); i++){
        rules.set(i, rules.get(i).replaceAll( "\\\\n","\n"));
    }
}

playGame() Method:

//is now a return type to return to my GUI
public static String playGame() {        
    String out;
    if(kings==3 && checkIfKing()==true){
        display("Player "+playerTurn+" has Drawn the Final King\n\n"+
                    getRule()+"\n\n",
                    "Restart the Program to Play again");
        System.exit(0);
        return null;
    }
    else if(checkIfKing()==true){
    kings++;
    playerTurn++;
    out="Player "+(playerTurn-1)+" has picked the "+deck.getCard().toString()
            +"\n"+(4-kings)+" Kings remain"
            +"\n\n"+getRule();     
    deck.removeFromDeck();
    if (playerTurn==players+1){playerTurn=1;}
    return out;
    }
    playerTurn++;
    out="Player "+(playerTurn-1)+" has picked the "+deck.getCard().toString()
            +"\n\n"+getRule();     
    deck.removeFromDeck();
    if (playerTurn==players+1){playerTurn=1;}
    return out;

}

checkIfKing() Method:

//Small but keeps my playGame() method cleaner.
public static boolean checkIfKing() {
    if(deck.getCard().rank==Rank.KING)
        return true;
    return false;
}

getRule() Method:

//Again, small but keeps my playGame() method cleaner.
private static String getRule(){
    return rules.get(deck.getCard().rank.ordinal());
}

skipTurn() Method:

//Just used to skip a turn and pass the message to the GUI
public static String skipTurn(){
    String out;
    if (playerTurn==players){
        out="Player "+playerTurn+"'s turn was skipped\n";
        playerTurn=1;
    }else{
        out="Player "+playerTurn+"'s turn was skipped\n";
        playerTurn++;
    }
    return out;
}

passCard() Method:

//A public method built specifically to pass card as a string to the GUI
public static String passCard(){      
    return deck.getCard().toString();
}

finalMessage() Method:

 //Just a legacy hanging around from the previous version, but it keeps my code clean.
 private static void finalMessage() {
    JOptionPane.showMessageDialog(null,"Player "+playerTurn+" has Drawn the Final King\n\n"+
                    getRule()+"\n\n",
                    "Restart the Program to Play again",1);
}


GUI

I made the GUI with the built in NETBEANS builder, which I've never used before. Deleted the snipped the starter comments

/**
 *-snip-
 * Project Path         :Kings/SourcePackages/JFrameKings/GameGUI.java
 * IDE                  :NETBEANS
 * -snip-
 */

Draw a Card Button on mouse pressed:

private void drawCardButton1MousePressed(java.awt.event.MouseEvent evt){                                             
    cardLogTA.append("Player "+JFrameKings.Kings.playerTurn+" drew the "
            + JFrameKings.Kings.passCard()+"\n");
    cardPickedTA.setText(JFrameKings.Kings.playGame());
    playerLabel.setText("Player "+(JFrameKings.Kings.playerTurn)+"'s turn");
}   

Skip Turn Button on mouse pressed:

private void skipTurnButtonMousePressed(java.awt.event.MouseEvent evt) {                                            
    String skip=JFrameKings.Kings.skipTurn();
    cardLogTA.append(skip);
    cardPickedTA.setText(skip);
    playerLabel.setText("Player "+(JFrameKings.Kings.playerTurn)+"'s turn");

}    

Exit Button on mouse CLICKED:

private void exitButtonMouseClicked(java.awt.event.MouseEvent evt) {                                        
    int sure=JOptionPane.showConfirmDialog(null, "Are you sure?");
    if (sure == 0){
    JOptionPane.showMessageDialog(null,"Bye!");
    System.exit(0);
    }
}
\$\endgroup\$
1
\$\begingroup\$

Why do you do? It seems redundant to me.

Collections.shuffle(cards, new Random());
Collections.shuffle(cards, new Random(System.nanoTime()));

I would not put labels of card ranks and other labels to the enums. I would use Properties stored in a file and used enum as a key in the file.

This

public static final int players=getNum("How many people are going to play",
                                       "Number of Players");

does not seem as a good idea. If i call new for a class i expect object to be created and initialized not some actions to be happening.

Indent your code!

if (playerTurn==players){
        out="Player "+playerTurn+"'s turn was skipped\n";
        playerTurn=1;
    }else{
        out="Player "+playerTurn+"'s turn was skipped\n";
        playerTurn++;
    }

Out is same in both branches.

I really dont like this String retur value sfor almost all methods. skipTurn should just skip turn. I dont see any reason why it should return anything.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the answer. As for the double shuffle, well I had a single shuffle but the cards seemed to always draw unrealistically, like a 15 turn games. The second shuffle fixed it, so I left it. Yeah the getNum() for the initialization is weird, I had it as a method I was going to use for additional menu options in the old GUI, it was already written and there so I just kinda left it for that one specific function. Whats wrong with my code indents? Also the if statement branches are necessary. The first branch resets the player turn if its the final player, the second iterates the turn. \$\endgroup\$ – Kylo Oct 8 '13 at 15:07
  • \$\begingroup\$ Also it does need to return a string value for the log of players turn so you can see if a turn was skipped. \$\endgroup\$ – Kylo Oct 8 '13 at 15:09
  • \$\begingroup\$ The playGame is not indented as it should be. Maybe it is only this forum formatting. Yes the if-else is necessary (or you could use ternary operator) but it is not necesary to assign the same value to variable out in if as in else. You could do ` out="Player "+playerTurn+"'s turn was skipped\n"; playerTurn = (playerTurn==players) ? 1 : playerTurn + 1` \$\endgroup\$ – DominikM Oct 9 '13 at 5:30
  • \$\begingroup\$ And game log is a View thing, it does not belong to the game itself. Ask for the game state in view, do not force your game logic to return game log. \$\endgroup\$ – DominikM Oct 9 '13 at 5:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.