Recursion/Looping Confusion

[Sw07fSEw]

I've been trying to write out a program to deal out a specified number of cards. Each card is represented by an index between 0 and 51, for a grand total of 52. Obviously the tricky part is making sure the computer doesn't deal out the same card twice. I wrote up code that I thought should work, but after staring at it for an hour, I can't find the logic error.

When the computer selects a random int from the deck of cards, that random int is checked to see if it's already been selected and added to a 'cardsDealt' vector via the push_back function. If the card hasn't been dealt previously, then it should be dealt. If it has been dealt, then obviously I want the computer to make a different selection and reevaluate that selection until it finds an index that hasn't been previously dealt. I'm sure there are more efficient ways to do this, however I'm still a beginner learning the fundamentals. Could I just get some input as to why my logic is wrong. My code is attached below. I have the program set to deal out 10 cards. It may take anywhere from 1 to 6-ish runs to get a duplicate.

#include <iostream>
#include <vector>
#include <cstdlib>
#include <string>
#include <ctime>
using namespace std;

vector<int> deck(52); // global vector deck
vector<int> _cardsDealt; //holds the cards dealt as vector

void genDeck(); // assigns a card to each index as an int
void dealCards();
void displayDealtCards();
int selectCard(vector<int> deck);

int main(){
	int cardsToDeal = 10;
	srand(time(NULL));
	genDeck();
	for(int i=0; i<cardsToDeal; i++){ //deals out 10 cards
		dealCards();
	}
	displayDealtCards();
	cout << "Number of cards to deal: " << cardsToDeal << endl;
	cout << "Acutual cards dealt: " << _cardsDealt.size() << endl;
	return 0;
}

void genDeck(){ // creates a vector of ints from 0 to 51, each card corresponds to an int
	vector<int> _deck(52);
	for(int i=0; i <deck.size(); i++){
		_deck[i] = i;
		deck[i] = _deck[i];
	}
}
int selectCard(vector<int> deck){ // randomly selects one of the indexes (cards), from the deck
	int card = rand() % 52;
	return card;
}
void dealCards(){
		int card = selectCard(deck); // calls selectCard function to select a random card
		cout << "The card selected is: " << card << endl;
		for(int n=0; n < _cardsDealt.size();n++){ //compares new selection to existing selections (indexes) in _cardsdealt vector
			if(card == _cardsDealt[n]){ // has the selected card already been dealt?
				cout << card << " HAS ALREADY BEEN SELECTED" << endl; 
				dealCards(); // since that card is in _cardsDealt vector, start over and select another random card
				break;
				cout << "IT SHOULD NEVER OUTPUT THIS" << endl;
			}
		}
		cout << card << " has been added to the vector" << endl; 
		_cardsDealt.push_back(card);	
}

void displayDealtCards(){
	cout << "The cards: ";
	for(int i=0; i <_cardsDealt.size(); i++){
		cout << _cardsDealt[i] << " ";
	}
	cout << "have been dealt" << endl;
}

When reading the output, I'm stumped. It'll output something like this:
The card selected is: 43
43 HAS ALREADY BEEN SELECTED
The card selected is: 16
16 has been added to the vector
43 has been added to the vector

Can anyone take a look at my code and explain why two different values for 'card' are being added to the vector? Intuitively I would think that calling the function again, which sets a different value for 'card', would overwrite the original, duplicate value, but apparently not.

[keanedawg]

Good commenting and explanation.

The problem is recursion.

You are in dealCards. When your if statement finds that card == _cardsDealt[n] is true,

It calls the following code,
dealCards(); // since that card is in _cardsDealt vector, start over and select another random card

I like your commenting style because it tells me exactly what your thinking. dealCards does not start over. It stacks on top of itself.

Think of it like this. You have two functions. You have function B and function A. Function A calls function B. What happens when function B ends? You return back to function A. Function A isn't reset, it's the exact same state as before.

Now suppose this is recursion. You call function A. Inside of function A, you call function A again. For clarity, Lets call it function A1 (first call of function A) and function A2 (second call of funcation A from funcation A1). When function A2 ends, you go back to function A1.

This is exactly what happens in your code. When you select 16, you are in function A2. But when you exit function A2, you go back to function A1 where you had selected 43. Thus you call cout the "added to vector" thing twice because there are two separate fucntions.

Hope this helps.

[Sw07fSEw]

Thanks for your feedback Keane, that really clarified the order in which the compiler is reading the code. I thought about what you said and made some changes to get the code to work. Is there anyway to tell the compiler to stop interpreting code past a certain point? In case this, when I call the function within itself, I don't want it to continue where it left off. Like you said, if I call A₁ and then call A₂ within A₁, is there anyway I can tell the compiler to 'forget' about completing A₁? Is that good practice or something to be avoided?

As of right now, I feel like I sort of cheated by using a placeholder, so to say, in line 8.

void dealCards(){
		int card = selectCard(deck); // calls selectCard function to select a random card
		cout << "The card selected is: " << card << endl;
		for(int n=0; n < _cardsDealt.size();n++){ //compares new selection to existing selections (indexes) in _cardsdealt vector
			if(card == _cardsDealt[n]){ // has the selected card already been dealt?
				cout << card << " HAS ALREADY BEEN SELECTED" << endl; 
				dealCards();// since that card has already been dealt, let's start over and select another random card
				card = 55; // assign 'card' a placeholder value so it doesn't automatically get 'pushed back'
				break;
			}
		}
		if(card < 52){
		cout << card << " has been added to the vector" << endl; 
		_cardsDealt.push_back(card);	
		}
}

[keanedawg]

Sorry, I was eating Turkey.

is there anyway I can tell the compiler to 'forget' about completing A1?

Your way seems okay to me. But you can't just tell the compiler to specifically exit A1. However, this should work.

I think this works. Let me know if it doesn't

void dealCards(){
		int card = selectCard(deck); // calls selectCard function to select a random card
		cout << "The card selected is: " << card << endl;
		for(int n=0; n < _cardsDealt.size();n++){ //compares new selection to existing selections (indexes) in _cardsdealt vector
			if(card == _cardsDealt[n]){ // has the selected card already been dealt?
				cout << card << " HAS ALREADY BEEN SELECTED" << endl; 
				dealCards(); // since that card is in _cardsDealt vector, start over and select another random card
                                // 

                                // INSERTION
				return; // Just return rather than break. 
                                // INSERTION
			}
		}
		cout << card << " has been added to the vector" << endl; 
		_cardsDealt.push_back(card);	
}

Your way looks fine as well. Maybe it's a little less readable but whatever works for you man.