Is this efficient code or can it be compressed

I am in my first year of coding in school. While on break I am continuing to learn C++ on my own. This is not a finished product due to the fact that I haven't wrote the function to keep score of the money earned or lost. I am trying to see if this code that I have written is efficient. Any input would be greatly appreciated.

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

void seedADeckOfCards(vector<int>&);
void dealFiveCardHand(vector<int>&, int [], char []);
void exchangeCards(int[], vector<int>&, int&, char []);
bool wantToSwapcards();
void outputDeck(const vector<int>&);

const int HAND = 5;

int main()
{
char suit[5];
int num = time(0);
srand(num);
vector<int> cardDeck;
int cardHand[5];

seedADeckOfCards(cardDeck);
std::random_shuffle(cardDeck.begin(), cardDeck.end());
dealFiveCardHand(cardDeck, cardHand, suit);

system ("PAUSE");
return 0;
}

void seedADeckOfCards(vector<int> &deckOfCards)
{
for(int i = 0; i < 52; i++)
deckOfCards.push_back(i+1);
}

void dealFiveCardHand(vector<int> &deckOfCards, int cardHand[], char suit[])
{
int card;
int dealFromBack = 51;
for(int i = 0; i < HAND; i++)
{
deckOfCards[dealFromBack] % 13 + 1;
cout << "CARD " << (i+1) << " = ";
cout << setw(3) << deckOfCards[dealFromBack] % 13 + 1 << " ";
suit[i] = deckOfCards[dealFromBack]%4+3;
cout << suit[i] << endl;
cardHand[i] = deckOfCards[dealFromBack] % 13 + 1;
deckOfCards.pop_back();
--dealFromBack;
}

exchangeCards(cardHand, deckOfCards, dealFromBack, suit);

cout << "Your new Hand is: \n";
for(int i = 0; i < HAND; i++)
{
cout << setw(3) << cardHand[i] << " ";
cout << suit[i] << endl;
}
cout << endl;
}

void exchangeCards(int fiveCardHand[], vector<int>& deck, int& sizeOfDeck, char suit[])
{
int howManyToSwap;
int cardToSwap, card;
vector<int> swapTheseCards;

if(wantToSwapcards())
{
do
{
cout << "How many cards do you want to swap? ";
cin >> howManyToSwap;
if(howManyToSwap < 1 || howManyToSwap > 5)
cout << "Invalid Entry!! Must be between 1-5\n";
}while(howManyToSwap < 1 || howManyToSwap > 5);



if(howManyToSwap > 1 && howManyToSwap < 5)
{
cout << "Enter the card(s) number (1-5) you want to exchange with a space between them. \n";
cout << "Swap card(s) number : ";

for(int i = 0; i < howManyToSwap; i++)
{
cin >> cardToSwap;
swapTheseCards.push_back(cardToSwap);
}
}
}

for(int i = 0; i < swapTheseCards.size(); i++)
{
card = deck[sizeOfDeck] % 13 + 1;
int num;
num = swapTheseCards[i];
num = num - 1;
fiveCardHand[num] = card;
suit[num] = deck[sizeOfDeck] % 4 + 3;
deck.pop_back();
--sizeOfDeck;
}
}

bool wantToSwapcards()
{
char answerSwap;
bool answer = false;
do
{
cout << "Do you want to swap any of your cards? (Y or N) ";
cin >> answerSwap;
if(!(answerSwap == 'y' || answerSwap == 'Y' || answerSwap == 'n' || answerSwap == 'N'))
cout << "Invalid entry enter Y or N ONLY!!\n";
}while(!(answerSwap == 'y' || answerSwap == 'Y' || answerSwap == 'n' || answerSwap == 'N'));

if(answerSwap == 'y' || answerSwap == 'Y')
answer = true;
}

void outputDeck(const vector<int>& deck)
{
for(int i = 0; i < deck.size(); i++)
{
cout << setw(3) << deck[i] % 13 + 1;
char suit = deck[i] % 4 + 3;
cout << suit << endl;
}
}
Edit your post and put [ Code ] [ /Code ] tags so we can see everything clearly, then we will let you know.
system ("PAUSE"); is considered poor form.

Also you're passing array's around from function to function. That means the entire array needs to be copied into a new memory space. You should pass using pointers. For example:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
void dealFiveCardHand(vector<int> &deckOfCards, int *cardHand[], char *suit[])
    {
        int card;        
        int dealFromBack = 51;        
        for(int i = 0; i < HAND; i++)        
        {
            deckOfCards[dealFromBack] % 13 + 1;
            cout << "CARD " << (i+1) << " = ";
            cout << setw(3) << deckOfCards[dealFromBack] % 13 + 1 << " ";
            suit[i] = deckOfCards[dealFromBack]%4+3;
            cout << suit[i] << endl;
            cardHand[i] = deckOfCards[dealFromBack] % 13 + 1;
            deckOfCards.pop_back();
            --dealFromBack;
        }
 }
Last edited on
ValliusDax wrote:
That means the entire array needs to be copied into a new memory space. You should pass using pointers.

Alas, that is not the case.

Arrays are passed by pointer -- the array name degenerates to a pointer, and the argument int cardHand[] is itself a pointer -- it could have been written with the equivalent int *cardHand.

In short, OP made no error there.
My bad Duoas... did not know that... **scribbles in note book**
Thanks for the info everyone. For james I reposted the code below.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
#include<iostream>
#include<cmath>
#include<cstdlib>
#include<string>
#include<vector>
#include<ctime>
#include<iomanip>
#include <algorithm>
using namespace std;

void seedADeckOfCards(vector<int>&);
void dealFiveCardHand(vector<int>&, int [], char []);
void exchangeCards(int[], vector<int>&, int&, char []);
bool wantToSwapcards();
void outputDeck(const vector<int>&);

const int HAND = 5;

int main()
{
    char suit[5];
    int num = time(0);
    srand(num);
    vector<int> cardDeck;
    int cardHand[5];
    
    seedADeckOfCards(cardDeck);
    std::random_shuffle(cardDeck.begin(), cardDeck.end());
    dealFiveCardHand(cardDeck, cardHand, suit);
   
    system ("PAUSE");
    return 0;
}

void seedADeckOfCards(vector<int> &deckOfCards)
{
     for(int i = 0; i < 52; i++)
     deckOfCards.push_back(i+1);   
}

void dealFiveCardHand(vector<int> &deckOfCards, int cardHand[], char suit[])
{
     int card;
     int dealFromBack = 51;
     for(int i = 0; i < HAND; i++)
     {
      deckOfCards[dealFromBack] % 13 + 1;
      cout << "CARD " << (i+1) << "  = ";
      cout << setw(3) << deckOfCards[dealFromBack] % 13 + 1 << "  ";
      suit[i] = deckOfCards[dealFromBack]%4+3;
      cout << suit[i] << endl;
      cardHand[i] = deckOfCards[dealFromBack] % 13 + 1;
      deckOfCards.pop_back();
      --dealFromBack;
     }
     
     exchangeCards(cardHand, deckOfCards, dealFromBack, suit);
     
     cout << "Your new Hand is: \n";
     for(int i = 0; i < HAND; i++)
     {
      cout << setw(3) << cardHand[i] << " ";
      cout << suit[i] << endl;
     }
     cout << endl;
}

void exchangeCards(int fiveCardHand[], vector<int>& deck, int& sizeOfDeck, char suit[])
{
     int howManyToSwap;
     int cardToSwap, card;
     vector<int> swapTheseCards;
     
     if(wantToSwapcards())
     {
      do
      {
       cout << "How many cards do you want to swap? ";
       cin >> howManyToSwap; 
       if(howManyToSwap < 1 || howManyToSwap > 5)
       cout << "Invalid Entry!! Must be between 1-5\n";
      }while(howManyToSwap < 1 || howManyToSwap > 5);
      
     
      
      if(howManyToSwap > 1 && howManyToSwap < 5)
      {
       cout << "Enter the card(s) number (1-5) you want to exchange with a space between them. \n";
       cout << "Swap card(s) number : ";
       
       for(int i = 0; i < howManyToSwap; i++)
       { 
        cin >> cardToSwap;
        swapTheseCards.push_back(cardToSwap);
       }
      }
     } 
   
    for(int i = 0; i < swapTheseCards.size(); i++)
    {     
     card = deck[sizeOfDeck] % 13 + 1;
     int num;
     num = swapTheseCards[i];
     num = num - 1;
     fiveCardHand[num] = card;
     suit[num] = deck[sizeOfDeck] % 4 + 3;
     deck.pop_back();
     --sizeOfDeck;
    }        
}

bool wantToSwapcards()
{
     char answerSwap;
     bool answer = false;
     do
     {
      cout << "Do you want to swap any of your cards? (Y or N) ";
      cin >> answerSwap;
      if(!(answerSwap == 'y' || answerSwap == 'Y' || answerSwap == 'n' || answerSwap == 'N'))
      cout << "Invalid entry enter Y or N ONLY!!\n";
     }while(!(answerSwap == 'y' || answerSwap == 'Y' || answerSwap == 'n' || answerSwap == 'N'));
     
     if(answerSwap == 'y' || answerSwap == 'Y')
     answer = true;
}

void outputDeck(const vector<int>& deck)
{
     for(int i = 0; i < deck.size(); i++)
     {
      cout << setw(3) << deck[i] % 13 + 1;
      char suit = deck[i] % 4 + 3;
      cout << suit << endl;
     }
}
Topic archived. No new replies allowed.