Deck of cards, trouble with inheritance?

Question

I am having a problem with inheritance.

Basically I am trying to teach myself object oriented programming and I can't get my enum array to use the toString method I have created for it. Instead it is using the one from the PlayingCards class.

How do I fix this?

public class PlayingCards {

private final Rank rank;
private final Suit suit;

 /**
 * Creates enums for rank
 */
 public static enum Rank {

    ACE(1),
    TWO(2),
    THREE(3),
    FOUR(4),
    FIVE(5),
    SIX(6),
    SEVEN(7),
    EIGHT(8),
    NINE(9),
    TEN(10),
    JACK(11),
    QUEEN(12),
    KING(13);
    private int rank;

    private Rank(int rank) {
        this.rank = rank;
    }

    public int getRank() {
        return this.rank;
    }

}

/**
 * Creates enums for suit
 */
public static enum Suit {

    HEARTS(14),
    SPADES(15),
    CLUBS(16),
    DIAMONDS(17);
    private int suit;

    private Suit(int suit) {
        this.suit = suit;
    }

    public int getSuit() {
        return suit;
    }

}

/**
 * Constructs a card with specified initial rank and suit
 *
 * @param rank sets rank to ACE by default
 * @param suit sets suit to SPADES by default
 */
public PlayingCards(Rank rank, Suit suit) {
    this.rank = rank;
    this.suit = suit;
}

public String toString() {
    return getClass().getName() + "[Rank = " + this.rank
            + ", Suit = " + this.suit + "]" + "\n";
}

public String format() {
    return this.rank + " of " + this.suit;
}

/**
 * Tests whether this card is equal to some other card.
 *
 * @param otherObject the card to be tested.
 * @param rank imports enums rank
 * @param suit imports enums suit
 * @return returns true if the suit and rank of test card is equal to the
 * suit and rank of this card otherwise false is returned.
 */
public boolean equals(Object otherObject, Rank rank, Suit suit) {
    if (otherObject == null) {
        return false;
    }
    if (getClass() != otherObject.getClass()) {
        return false;
    }
    PlayingCards other = (PlayingCards) otherObject;
    return suit == other.suit && rank == other.rank;
}

}

This section is where I am having problems. It is creating the array fine but it's just displaying it in the compiler using the toString method from the wrong class.

import java.util.Arrays;

public class Pack extends PlayingCards
{
 static PlayingCards[] card = new PlayingCards[52];
 PlayingCards.Suit[] suit2 = PlayingCards.Suit.values();
 PlayingCards.Rank[] rank2 =  PlayingCards.Rank.values();
 private int numberOfCards;
 /**
 * Constructs a pack of 52 cards.
 * Sorted by suit Clubs, Diamonds, Hearts, Spades.
 * Sorted ascending.
 * @param rank
 * @param suit
 */  
 public Pack(Rank rank, Suit suit)
 {
 super(rank,suit);
    card = new PlayingCards[52];
    numberOfCards = 0;
    for ( int x = 0; x < suit2.length; x++ )
    {
        for ( int y = 0; y < rank2.length;y++ )
        {

            card [numberOfCards] = new PlayingCards(rank2[y],suit2[x]);
            numberOfCards ++;
        }
    }
}

/**
* Shuffles cards in pack.
*/
 public void shuffle()
 {
 }

/**
* @return string representation of 52 card pack.
*/

@Override
public String toString() {
String toString = "New pack\n";
    for (int cards =0; cards < card.length; cards++)
    {
        toString = toString + card[cards] + "\n";
    }
    return toString;
}
}

This is what I get. The result has been formatted using the wrong toString method:

 assed3.PlayingCards[Rank = ACE, Suit = HEARTS]
 , assed3.PlayingCards[Rank = TWO, Suit = HEARTS]
 , assed3.PlayingCards[Rank = THREE, Suit = HEARTS]
 , assed3.PlayingCards[Rank = FOUR, Suit = HEARTS]
 , assed3.PlayingCards[Rank = FIVE, Suit = HEARTS]

Answer 1

Problems/errors in your code:

• Naming. The class PlayingCards should be renamed as PlayingCard. An instance of this class will represent one playing card, not many. You want to name your classes with names that represent the objects they are modelling in the real world. This avoids confusion when programming with these classes and makes code more intuitive.

• Is-a relationship between superclass and subclass. A deck of cards is not a playing card. You are wrong to let the Deck class inherit from the PlayingCard class, it makes no sense at all. A deck of cards is composed of playing cards. In this case, drop the inheritance relationship between Deck and PlayingCard.

If you apply these two, your problem will likely be solved already.

---

Other remarks:

• The equals method should be overriden from Object.equals. The equals method you have now in the PlayingCards class doesn't make any sense. The easiest way to implement an override is to ask your IDE to generate an implementation for you. When you work in Eclipse for instance, you can ask it to generate it for you. In almost all cases the provided implementation will be OK.

• Whenever you have overriden equals, you should generally (read: always) also override Object.hashCode: equal objects must have equal hash codes. You can usually also ask your IDE to provide an implementation for you.

• When you override a method from the superclass, always add the @Override annotation to the method. This will clearly signal that the method is an override. This may not be super important for an override of Object.toString because everyone knows that method, but in other classes for different methods it makes code reviewing a lot easier. A method with an @Override annotation is most definitely an override. A method without that annotation might be or might not be, depending on the superclass. A method that doesn't override a method in the superclass with the @Override annotation will result in a compiler error. The more clarity, the better.

• Having the suit represented by an integer is silly. Have the suit represented by a String: "Hearts", "Clubs", "Diamans", "Spades".

• Having PlayingCards.Suit[] suit2 and PlayingCards.Rank[] rank2 as members of the Deck class is a waste of memory. If you want to assign ranks or suits to your PlayingCards[] card member, use PlayingCards.Suit.values() and PlayingCards.Rank.values() directly in your constructor.

• int numberOfCards is a member variable you use in the constructor as a loop variable. You should not declare this as a member, but should declare it as a local variable in your constructor. That is not to say that you can't have a member representing the total number of cards in the deck, but you shouldn't use it as a loop variable.

• You append \n to strings for newlines, but some operating systems expect \r\n for line separators. You can get the proper line separator for the OS you are running on using System.getProperty("line.separator").

• Building strings is best done using the StringBuilder class. The way you construct the string in toString representing the deck, creates a new String in each iteration. That way of working consumes a lot of memory and trashes the heap leading to fragmentation.

• In your class setup now, with Deck deriving from PlayingCards you are incorrectly assuming that when PlayingCards.toString is called from the Deck.toString implementation, that the getClass().getName() will return the class name for class Deck. At least that's what I think you are expecting. Since you are calling PlayingCards.toString on a PlayingCards instance, getClass().getName() will return PlayingCards prepended by the package name.

Answer 2

i can't get my enum array to use the toString method

You should say exactly what is happening. Usually this sort of bug can be diagnosed using your debugger but one problem you have is

for (int cards =0; cards <=card.length; cards++) 

which should be

for (int cards =0; cards < card.length; cards++)

and if this is the case, you would be getting an error showing what the problem is and exactly which line has the error.