Deck of cards design

Can someone please review this design I created for deck of cards in Java?

```
package prep.design;

import java.lang.reflect.Array;
import java.util.*;

/**
* Created by rohandalvi on 5/14/16.
*/
public class CardDeck {

private ArrayList cards;

CardDeck(){
cards = new ArrayList<>();
for(Card.Suit s: Card.Suit.values()){
for(Card.Rank r: Card.Rank.values()){
cards.add(new Card(r,s));
}
}
}

public void print(){
for(Card card: cards){
System.out.println("Card: "+card.getRank()+" Suit: "+card.getSuit());
}
}

public void shuffle(ArrayList c){
if(c==null){
Collections.shuffle(cards);
}else{
Collections.shuffle(c);
}

}

public static void main(String[] args) {
CardDeck cd = new CardDeck();
cd.shuffle(null);
//cd.print();

cd.deal(5);
}

public void deal(int numberOfPlayers){
ArrayList tempDeck = new ArrayList<>(cards);
Map> playerDeck = new HashMap<>();
//List> playerDeck = new ArrayList<>();
shuffle(tempDeck);
shuffle(tempDeck);

int i=0;
while(i!=52){
int j = i%numberOfPlayers;
List tempList;
if(playerDeck.containsKey(j)){
tempList = playerDeck.get(j);
}else{
tempList = new ArrayList<>();
}
tempList.add(tempDeck.get(i));
playerDeck.put(j,tempList);

i++;

}

System.out.println("Dealt");

displayPlayerCards(playerDeck);

}

public void displayPlayerCards(Map> playerDeck){
int i=0;
for(Integer player: playerDeck.keySet()){
List playerCards = playerDeck.get(player);

System.out.println("Player "+i);
for(Card c: playerCards){
System.out.print("Rank: "+c.getRank()+" Suit: "+c.getS

Card class

Specify the access modifiers of the class and members properly.
Use proper names for members:

class  Card{
    Suit s;
    Rank r;

for example:

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

print() method

It's better to only create a String representation of an object and return that. You override toString() method which will be used implicitly if the object has to be converted to a String.

It's good practice to override toString() for all custom classes, because this is where people will look for when searching for a String representation of the object. You called it print(), or was it info()? , who knows? Your future self? Probably not. toString() is the way to go.

Your current approach appears to be more convenient, but actually causes some trouble like duplicated code:

System.out.print("Rank: "+c.getRank()+" Suit: "+c.getSuit()+"\t");
System.out.println("Card: "+card.getRank()+" Suit: "+card.getSuit());

Card.toString() could look like this:

@Override
public String toString()
{
    return "rank: " + rank + "\t suit: " + suit; 
}

CardDeck is pretty much a collection of a number of cards. CardDeck.toString() could look something like this:

@Override
public String toString()
{
    String result = cards.size() + " cards:" + System.lineSeparator();
    for (Card card : cards)
    {
        result = result.concat(card + System.lineSeparator());
    }

    return result;
}

Here's what both classes look like so var with a little example program:

Card.java

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

    public  enum  Suit
    {
        Spade , Heart , Diamond , Clubs
    }

    public 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);

        int priority;

        Rank(int s) 
        {
            priority = s;
        }

        public int getPriority()
        {
            return priority;
        }

        public Rank getRankByPriority(int p)
        {
            switch (p)
            {
                case 1: return Rank.ACE;
                case 2: return Rank.TWO;
                case 3: return Rank.THREE;
                case 4: return Rank.FOUR;
                case 5: return Rank.FIVE;
                case 6: return Rank.SIX;
                case 7: return Rank.SEVEN;
                case 8: return Rank.EIGHT;
                case 9: return Rank.NINE;
                case 10: return Rank.TEN;
                case 11: return Rank.JACK;
                case 12: return Rank.QUEEN;
                case 13: return Rank.KING;

                default: return null;
            }
        }
    }

    Card(Rank rank, Suit suit)
    {
        this.rank = rank;
        this.suit = suit;
    }

    Rank getRank()
    {
        return rank;
    }

    Suit getSuit()
    {
        return suit;
    }

    @Override
    public String toString()
    {
        return "rank: " + rank + "\t suit: " + suit; 
    }
}

CardDeck.java

import java.util.ArrayList;

public class CardDeck 
{
    private ArrayList cards;

    CardDeck()
    {
        cards = new ArrayList<>();

        for(Card.Suit s: Card.Suit.values())
        {
            for(Card.Rank r: Card.Rank.values())
            {
                cards.add(new Card(r,s));
            }
        }
    }    

    public static void main(String[] args)
    {
        CardDeck deck = new CardDeck();

        System.out.println(deck);
    }

    @Override
    public String toString()
    {
        String result = cards.size() + " cards:" + System.lineSeparator();
        for (Card card : cards)
        {
            result = result.concat(card + System.lineSeparator());
        }

        return result;
    }
}

result (excerpt):

$ java CardDeck 
52 cards:
rank: ACE    suit: Spade
rank: TWO    suit: Spade
rank: THREE  suit: Spade
rank: FOUR   suit: Spade
rank: FIVE   suit: Spade
rank: SIX    suit: Spade
rank: SEVEN  suit: Spade
rank: EIGHT  suit: Spade
rank: NINE   suit: Spade
rank: TEN    suit: Spade
...

shuffle() method

A method is a block of code that operates on an object. Your shuffle() method might work on an optional parameter which is entirely independent from the CardDeck object. The part of the method that operates on an arbitrary ArrayList should be static.

public void shuffle(ArrayList c){
        if(c==null){
            Collections.shuffle(cards);
        }else{
            Collections.shuffle(c);
        }

    }

Thus splits into:

public static void shuffle(ArrayList cards)
{
    if(cards!=null)
    {
        Collections.shuffle(cards);
    }
}

public void shuffle()
{
    Collections.shuffle(cards);
}

The second one makes a lot of sense, but the static version does not. What's the point of calling this method directly when you already have Collections.shuffle()?

StackExchange Code Review Q#128398, answer score: 11