C++

Forum

 
Is this code efficient

OUIJ (50)
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;
}
}
closed account (N36fSL3A)
The use of system(x) is not recommended and can attract the unwanted attention of AV.
closed account (91qLy60M)
Lumpkin, sir, are you sure about the anti-virus? I've always read it was recommended not to use system("PAUSE") because not all OSes have a PAUSE option. I read it was better to just do getchar() or such to make the program hang. Sir, can you site where it says it attract AV attention because that sounds rather odd.
ResidentBiscuit (4459)
system() in general is frowned upon. There are lots of articles out there explaining why if you're curious.

Anyways, if you want your code looked at more it's wise to place it in code tags. They are under Format: by the text box when submitting a post.
Duthomhas (13114)
@Phoenix Omega
Problems with system(): http://www.cplusplus.com/articles/j3wTURfi/

As for your code, you could have reduced typing a little, but in terms of efficiency, it's fine.

Unless you plan to have another program try to play a gazillion hands in less than a second.

(The limiting factor here is the user's response time, which is significantly longer than anything your program will do. As long as the program feels sufficiently responsive to the user -- and doesn't waste resources -- it is fine.)

Hope this helps.
OUIJ (50)
Thanks everyone for the input and I will look into the system() situation causing AV.
For those who wanted the code tags here it is again with the tags
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
137
 
#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;
     }
}
closed account (N36fSL3A)
Using system(x) sends AVG crazy most of the time (at least on my machine).

http://www.cplusplus.com/articles/j3wTURfi/
OUIJ (50)
Lumpkin what would you recommend for the replacement of system("CLS")
and I know you can do a get.ch(). to replace system("PAUSE")
long double main (999)
There's a nice article on clearing the screen here:

http://www.cplusplus.com/articles/4z18T05o/
closed account (91qLy60M)
Lumpkin and Duoas, sirs, I thought that was a myth. My cousin is getting into programming with Bloodshed Dev-C++, but the included examples there use system() calls, but his Norton, McAfee, nor AVG pop up when executing them. He set up Ubuntu Linux for me so I will have to keep that fact in mind. Thank you, sirs.
closed account (N36fSL3A)
Just use cin.get();
closed account (Dy7SLyTq)
well, it isnt generally a good one. it can work, but only if the buffer is clear. im sure there is something on windows that will achieve the same effect, but on linux what i do using a class i found (and edited a bit) i clear the buffer and then take unbuffered input

edit: appending to my post... at that point it is safe to use cin.get() as it will have to actually wait for the use to hit the keyboard again
Last edited on
closed account (N36fSL3A)
Doesn't std::endl clear the buffer?
closed account (Dy7SLyTq)
it flushes it (source http://en.cppreference.com/w/cpp/io/manip/endl ) although i dont know if there is a difference. it would flush the output buffer not what the person is inputting (or at least thats what i think would happen. ill do some experimenting)
closed account (Dy7SLyTq)
using the program:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
#include <iostream>

using std::cin;
using std::cout;
using std::endl;

int main(int argc, char *argv[])
{
    char i=0;
    cout<<"$> ";
    cin>> i;
    cout<< endl;
    cin.get();
}


i get this (input in bold):

$> ff


and... it did not pause. that was a bit dramatic :P i hadnt tested it before writing this so i was in suspense
Duthomhas (13114)
The 'not clear' bit of the buffer is half the question.

First, your program should manage input properly -- that is, it should read input to properly synchronize what the user gives it.

Second, this is why I advocate using cin.ignore( numeric_limits <streamsize> ::max(), '\n' ) instead of just cin.get() or some other simple thing.

If you really want to 'clear' or 'flush' the input stream, which is a Bad Idea -- because it forces user agents to have to handle your program specially -- there are OS-specific ways to test for ready input and read it.
OUIJ (50)
Thank you to everyone for their input. It is greatly appreciated. I am going to create a new post to discuss another issue I have with the same program.
Topic archived. No new replies allowed.