Please help me fix this

[keygiwawah]

I am making a deck of cards and then trying to shuffle them.

Here is my class for a card.

public class Card{
	int rank,suit;
	
	public Card(int s, int n){ 
		rank = n;
		suit = s;
	}
	
	public int getRank(){
		return rank;
	}
	
	public void setRank(int n){
		rank = n;
	}
	
	public int getSuit(){
		return suit;
	}
	
	public void setSuit(int s){
		suit = s;
	}

	public String toString(){
		String asdf = "";
		String fdsa = "";
		
		if(getSuit() == 0){
			asdf += "Spades";
		}
		else if(getSuit() == 1){
			asdf += "Diamonds";
		}
		else if(getSuit() == 2){
			asdf += "Clubs";
		}
		else{
			asdf += "Hearts";
		}
		
		
		if(getRank() == 0){
			fdsa += "2";
		}
		else if(getRank() == 1){
			fdsa += "3";
		}
		else if(getRank() == 2){
			fdsa += "4";
		}
		else if(getRank() == 3){
			fdsa += "5";
		}
		else if(getRank() == 4){
			fdsa += "6";
		}
		else if(getRank() == 5){
			fdsa += "7";
		}
		else if(getRank() == 6){
			fdsa+= "8";
		}
		else if(getRank() == 7){
			fdsa += "9";
		}
		else if(getRank() == 8){
			fdsa += "10";
		}
		else if(getRank() == 9){
			fdsa += "Jack";
		}
		else if(getRank() == 10){
			fdsa += "Queen";
		}
		else if(getRank() == 11){
			fdsa += "King";
		}
		else if(getRank() == 12){
			fdsa += "Ace";
		}
		
		return fdsa + " of " + asdf;
	}
}

Here is a class for the deck.

public class Deck{
		
		Card[] theDeck;  //decalres an array of integers.
		Card[] deckShuffled;  //decalres an array of integers.
		Card[] pickedNums;  //decalres an array of integers.
		
	public Deck(){

		theDeck = new Card[52];  //allocates memory for 52 cards.
		//This is the deck untouched.
		
		int x = 0;
		
		for(int i=0; i<4; i++){
		
			for(int c=0; c<13; c++){
				
				theDeck[x] = new Card(i,c);
				System.out.println(theDeck[x]);
				x++;
				
			}
		}
		//This has just created all of the cards in order.
		
		deckShuffled = new Card[52];  //allcates memory for 52 cards.
		/*This is where the shuffled deck will be, cards are randomly placed
		here from the deck.*/
		
		pickedNums = new Card[52];  //allocates memory for 52 cards.
		/*This will contain the cards that were transfered from deck to 
		deckShuffled and will be used to stop cards from being used multiple times.
		*/
		
		int[] PickedNums = new int[52];
		for(int i =0; i<PickedNums.length; i++){
			PickedNums[i] = -1;
		}
		
		int pickedNumsIndex = 0, shuffledNumsIndex = 0, num = 0;
		boolean hasBeenPicked = true;
		while(shuffledNumsIndex<=51){
			System.out.println("shuffled nums index: "+shuffledNumsIndex);
			while(hasBeenPicked){
				num = (int)(Math.random()*52.0);
				System.out.println(num);
				for(int c=0; c <= pickedNumsIndex; c++){
					if(num == PickedNums[c]){
						System.out.println("num has been found at: "+c);
						hasBeenPicked = false;
					}
					else{
						hasBeenPicked = true;
						break;
					}
				
				}
			}
			hasBeenPicked = true;
			PickedNums[pickedNumsIndex] = num;
			pickedNumsIndex++;
			
			deckShuffled[shuffledNumsIndex] = theDeck[num];
			shuffledNumsIndex++;
		}
	theDeck = deckShuffled;
		
	}
	
		
		
}

I am having trouble with my deck shuffling. It seems to be stuck in an infinate loop, and I am not sure how to fix it. I printed out num to show the infinate loop, you can just delete that if you don't need it. Please show me how to fix it using my code, not by introducing entirely new concepts. I am new to java and may not be using the most eficient methods to solve problems.



Here is my driver.

public class DeckDriver{
	public static void main(String[]args){
		
		Deck asdf = new Deck();
	}
}

[masijade]

Use a List instead of an array then use Collections.shuffle

[collin389]

or just convert your array to a list and shuffle that. If you don't want to change all your code to a list.

[rdtindsm]

OP explicitly does not want "new concepts", i.e. collections, lists, etc.

If you want to keep the code at a more basic level an effective short routine to shuffle is:
[code]
for (i = 0:51){
swap card[i] with card[random]
end for
}[/ code]

only one pass, don't have to check to see if card is pick more than once. You don't really care. Because every slot is visited, every card is guaranteed to be moved, either as a source or destination in a swap. It is allowable to shuffle a card back to the same slot. In place, so memory usage is strictly n

Also suggest just using the numbers 0-51 as cards, then converting using suit = n/13, rank = n % 13; I realize that the purpose of the exercise may be to learn the techniques you are using. That's great.

[rdtindsm]

Just got out of the eclipse debugger a few minute ago.

I think if you sit down with a piece of paper and walk through this loop by hand:
pick a random number, then walk through this loop by hand.
for(int c=0; c <= pickedNumsIndex; c++){....}
picking a random number, and comparing it with what is actually in the array, you will find
your problem very quickly. If you haven't found it by the time you get to c=1, you're doing something wrong.

Ok, so c won't actually increment the way it's written. Let me repeat, you should find the problem very quickly.
When you do, your code seems to work.

Be sure to write out completely the contents of the PickedNums array and update it as needed.
Hint: You have a println in this loop. It never prints.

Also, you have arrays labeled pickedNums and also PickedNums, and seem to be using them both. Confusing as
hell.

When you figure out what's wrong, rewrite the code. If you are turning this in for a grade, the evaluator may give you full credit
when it works, but why not impress him? I wrote this program as one of the exercises in the java Tutorials and I did it exactly the way you are doing it. Not good.

[masijade]

Who says the OP "didn't want new concepts"? He asked how to shuffle a Deck and we gave him a much easier, and probably more effecient, way of doing it. Had the OP said, "according to the assignment instructions I must use an array", or he were to come back and say that, then yes, you would be right. Until then, that advice, and that code, is alot of nothing. It is simply recreating the wheel, seemingly for the joy of it. Are you the type that builds your own wooden wheels to replace the tires on your car?

[Tolls]

Well, yes he didn't say that explicitly, but you must admit that most of the things we get here are learning exercises that have less to do with Java than with coding in general. And they're generally pretty easy to spot.

I suppose the better answer would be along the lines of:
"assuming you can't simply use Collections.shuffle()..."

[masijade]

Yes, and I do admit that, but I also acknowledge that many times they are allowed to use other things in that the assignment does not restrict it, but that is all they have used to this point. I will point out the right answer first. If the OP then comes back and says "the assignment says I can't", then I will help them the other way. But I definately will not say to other posters "don't give the right answer, simply get the wrong way to work right" without that statement from the OP. And that is what I was responding to.

[Tolls]

No, fair point.
We just take things from a different stand point...here I don't view many of the questions to be Java per se, even though this is a Java forum. They're often simply algorithmic exercises.

One of the reasons I tend to stick to the ones that are clearly not school questions. At least if someone's doing JDBC then they damn wel ought to know how the Java API works.

[rdtindsm]

from masijade
Who says the OP "didn't want new concepts"?
from OP
Please show me how to fix it using my code, not by introducing entirely new concepts. I am new to java and may not be using the most eficient methods to solve problems.

from masijade
But I definately will not say to other posters "don't give the right answer, simply get the wrong way to work right" without that statement from the OP.

I've been personally frustrated more than once when somebody insisted on telling me the "right" way rather than what I was doing wrong.

I was guilty of the same thing by introducing another algorithm. And I also took the trouble to find his problem and tell him where to look. A lot of the posters need to learn basic techniques of programming and trouble shooting, as was the case here. This is the basic skill that needed to be addressed first. Then point out the better way. I get the feeling the OP felt the same way.

Don't mean to be criticizing the OP. His code was well structured and showed competence. We all have problems that we get stuck on. I am sure he will learn and effectively use some of these more advanced techniques later, and it is certainly a good idea to introduce better alternatives. It will make it more relevant when he gets there.

But using a different method just sweeps his original problem under the rug. It increases his knowledge without increasing his skill.

[Fubarable]

I think that all would be moot if the OP were to actively join in to this discussion, but instead his absence is noteworthy and disconcerting.

[masijade]

No it doesn't. Why bother doing it, unless it is just to learn the technique? And if that's the case then you need to say that. If you want to do it that way anyway, then be my guest, but don't criticise others for giving the right answer. Go ahead and keep creating wooden wheels, but don't criticize others for buying those fancy rubber things.