HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Console Card Game - Contract Bridge

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
contractcardbridgegameconsole

Problem

My project has three classes: Card, Deck, and BridgeConsole. The Card class has two fields: int rank and int suite. It has three methods: toString, getSuitAsString, and compareTo:

public class Card implements Comparable{
int suit; // 0-Hearts, 1-Clubs, 2-Diamonds, 3-Spades
int rank; // 2-10, 11-Jack, 12-Queen, 13-King, 14-Ace

public Card(int rank, int suit) {
    this.rank = rank;
    this.suit = suit;
}

public int getRank() {
    return rank;
}

public int getSuit() {
    return suit;
}

public String getSuitString() {
    String str = null;
    switch(suit){
        case 0: 
            str = "Hearts";
            break;
        case 1: 
            str = "Clubs";
            break;
        case 2:
            str = "Diamonds";
            break;
        case 3:
            str = "Spades";
            break;
        default:
            System.out.print("Fatal error!!!");
            break;
    }
    return str;
} 

@Override
public String toString() {
    return rank + " of " + getSuitString() + ", ";
}
@Override
public int compareTo(Object o) {
    Card c2 = (Card) o;
    if( suit  c2.suit) {
        return 1;
    }
    if( rank > c2.rank) {
        return 1;
    }
    if( rank  c2.rank;
    }
}


My Deck class constructs the deck of cards and has a method to deal cards to the players:

public class Deck {
    ArrayList deck = new ArrayList<>();

public Deck() {
    for (int s = 0; s  0) {
        card = deck.remove(deck.size() - 1);
    }
    return card;
    }

}


Finally, the game logic and main method are contained in the BridgeConsole class. I used modulo arithmetic to keep track of the previous rounds winner so as to start with that player in the subsequent round.

```
public class BridgeConsole {

static Scanner stdin = new Scanner(System.in);
static Deck deck = new Deck();
static int playerCard;
static ArrayList player = new ArrayList<>();
static ArrayList ai1 = new ArrayList<>();
static ArrayList ai2 = new ArrayL

Solution

Everywhere you are using ArrayList in variable and method parameter declarations.
It's a recommended practice to use the interface type whenever possible,
in this case List. So for example instead of:

ArrayList deck = new ArrayList<>();


Write as:

List deck = new ArrayList<>();


In the BridgeConsole class everything is static.
This can make unit testing hard,
and it's quite unusual.
It would be better to change everything to non-static,
as much as possible.
You might want to separate into two classes:
one which manages a game state,
and one which just starts running the whole thing (with the main method).

For modeling suits in card games, a more common approach is to use an enum.

enum Suit {
    Hearts,
    Clubs,
    Diamonds,
    Spades
}


In a switch statement,
when multiple cases have the same body,
you can group them together.
So instead of this:

switch (winner) {
    case 0:
        team1++;
        break;
    case 1:
        team2++;
        break;
    case 2:
        team1++;
        break;
    case 3:
        team2++;
        break;
}


You could simplify to:

switch (winner) {
    case 0:
    case 2:
        team1++;
        break;
    case 1:
    case 3:
        team2++;
        break;
}

Code Snippets

ArrayList<Card> deck = new ArrayList<>();
List<Card> deck = new ArrayList<>();
enum Suit {
    Hearts,
    Clubs,
    Diamonds,
    Spades
}
switch (winner) {
    case 0:
        team1++;
        break;
    case 1:
        team2++;
        break;
    case 2:
        team1++;
        break;
    case 3:
        team2++;
        break;
}
switch (winner) {
    case 0:
    case 2:
        team1++;
        break;
    case 1:
    case 3:
        team2++;
        break;
}

Context

StackExchange Code Review Q#91529, answer score: 6

Revisions (0)

No revisions yet.