I've wrote this method on my own I want to know if there is a better way to do it?
public Card[] shuffle(){
for(int i = 0; i < Deck.length; i++){
int x = i + (int) (Math.random() * (32 - i));
Card temp = Deck[i];
Deck[i] = Deck[x];
Deck[x] = temp;
}
return Deck;
}
You not checking, if ( 32 - i )
ever gives any value less than 0
. The algorithm is called Fisher-Yates shuffling algorithm, which resemble very much like yours:
private int [] shuffleMyArray ( int [] array ) {
int size = array.length, i = 0;
int temp = 0;
while ( size != 0 ) {
i = ( ( int ) ( Math.random () * size-- ) );
if ( i < 0 ) {
i = 0;
}
temp = array [ size ];
array [ size ] = array [ i ];
array [ i ] = temp;
}
return array;
}
EDIT 1:
The output of both the algorithms will better make you understand the difference between the two, see how Fisher-Yates
take all indices into account, while shuffling.
OUTPUT:
Actual Array
0 1 2 3 4
Your Implementation output
i: 0 x: 3
i: 1 x: 4
i: 2 x: 3
i: 3 x: 4
i: 4 x: 4
Fisher Yates implementation output
i: 4 size: 4
i: 2 size: 3
i: 1 size: 2
i: 0 size: 1
i: 0 size: 0
I'm going to make the deck
an argument, and the method static
. That way it's self contained. In Java, naming conventions are to make variables and method names start with a lower case letter. Next, the shortest way I know is to shuffle the deck in place with Collections.shuffle(List)
and Arrays.asList(T...)
like
public static void shuffle(Card[] deck) {
Collections.shuffle(Arrays.asList(deck));
}
If you want to preserve the original deck
, then you could copy it and return
like
public static Card[] shuffle(Card[] deck) {
List<Card> al = new ArrayList<>(Arrays.asList(deck));
Collections.shuffle(al);
return al.toArray(new Card[deck.length]);
}
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With