Clean up code

I was told to clean up the code and make it easier to understand but all the teacher did was confused me. Any tips? thanks.

These are the steps he asked me to take:


-Change variable and function names to camelCase and remove underscores __

-Make all variable names reflect their purpose (no i's, n's, etc.).

-Remove all global variables and Magic Numbers.

-Change the character arrays to be either arrays or vectors of strings.

-Remove the need for a break statement.

-Validate the number of cards entered by the user to draw from the deck is between 1 and 52.

-Program should ask the user to whether to repeat and not base the decision on invalid input for the number of cards.

-Clearly comment all functions and algorithms.

-Format and indent the code so that the layout reflects the flow.

-Any additional changes to make the code safer and more secure.

-Any additional change to make the code less likely to have bugs and easier to maintain.

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
  #include <iostream>
#include <cstdlib>
#include <ctime>
#include <cmath>
using namespace std;

int rand_0toN1(int n);
void draw_a_card();
int select_next_available(int n);
bool card_drawn[52];
int cards_remaining = 52;

char *suits[4] =
		{"hearts", "diamonds", "spades", "clubs"};

char *ranks[13] =
		{"ace", "two", "three", "four", "five",
			"six", "seven", "eight", "nine",
				"ten", "jack", "queen", "king" };
int main() 
{
int n, i;
srand(time(NULL));  
while (1) 
{
	cout << "Enter no. of cards to draw ";
	cout << "(0 to exit): ";
	cin >> n;
	if (n == 0)
		break;
	for (i = 1; i <= n; i++)
		draw_a_card();
}
return 0; 
}

void draw_a_card() 
{
int r, s;
int n, card;
n = rand_0toN1(cards_remaining--);
card = select_next_available(n);
r = card % 13;          
s = card / 13;     
cout << ranks[r] << " of " << suits[s] << endl;
}

int select_next_available(int n) 
{
int i = 0;
while (card_drawn[i])
i++;
while (n-- > 0)
{     			
i++;         
while (card_drawn[i])
i++;
}
card_drawn[i] = true;
return i; }

int rand_0toN1(int n) {
	return rand() % n; }
Last edited on
What is so confusing to you?

It looks like you're saying "I can't do this. Do it for me" which is, I'm sure, not the impression you meant to make.
Here, let me show you the difference. You need to name things what they are, if your variable is for numbers, don't name it 'n', name it numbers.
Example:
1
2
int n = 0; // No
int numbers = 0; // Yes 


Comment EVERYTHING (practically), meaning comment things that are being done, not something like ' i = 0; // Set i to zero '
Example:
1
2
3
4
while(getline(fileName, line) // Obtaining lines as strings from file
{
  // Some code here.
}


Underscores are the old method to programming, I personally still use them because they make sense to me, however, camel case is the more modern method. Camelcase is similar to this: 'int number_var' becomes 'int numberVar', its pretty simple.

Char arrays are an older method, there is nothing wrong with them, they work. A vector is dynamic, I like vectors much better, they work and are easy to use. Look into vectors, its much easier to use. Adding something to the vector is as simple as: vector.push_back(variable);
Example:
1
2
3
4
int myArray[number];
vector<int> myVector; 

myVector.push_back(number); // Adds number to vector array 


This is one of my codes, compare it to your own and see how easy it is to read.

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
#include <iostream>
#include <time.h>
#include <stdlib.h>
#include <vector>

using namespace std;

const int Error = 1; // Error exit

void printList(vector<int> numbers); // Prototypes
void restart(int option);

int main()
{
 srand(time(NULL)); // Random Numbers on Time

 vector<int> numbers;
 int num=0, choice=0, option=0;

 cout << " Playing the Numbers" << endl;           // Player Menu
 cout << "---------------------" << endl << endl;

 cout << " What would you like to do?" << endl;
 cout << " 1) Pick 5" << endl;
 cout << " 2) Exit" << endl << endl;
 cout << " Choice: ";
 cin  >> choice;

 switch(choice)
  {
   case 1: while(numbers.size() < 5) // Keep size at 5 numbers
            {
             num = rand();
             if(num > 59)
              {
               continue;
              }
             else 
              {
               numbers.push_back(num); // Insert Random Number < 59
              }
            }//end while
           break;

   case 2: cout << "\n Exiting..." << endl;
           system("pause");
           exit(Error);
           break;

  }//end switch

 printList(numbers); // Print out Numbers

 cout << endl;

 restart(option); // Re-Pick
   

cin.ignore();
cin.get();
return 0;
}

void printList(vector<int> numbers) // Prints list of random numbers
{
 cout << "\n\n 5 Pick!" << endl << endl;
 for(unsigned int i=0; i<numbers.size(); i++)
  {
   cout << " " << numbers[i];
  }
 cout << endl;

}

void restart(int option) // Restart Menu
{
 cout << " Would you like to go again?" << endl;
 cout << " 1) Yes\n 2) No" << endl << endl;
 cout << " Choice: ";
 cin  >> option;

 if(option == 1)
  {
   system("cls"); // Windows Only clear screen
   main();
  }
 else if(option == 2)
  {
   cout << "\n Exiting..." << endl;
   system("pause"); // Windows Only pause
   exit(Error);
  }
 else cout << " Wrong Choice!" << endl << endl;
      system("pause"); // Windows Only pause
      system("cls");  // Windows Only clear screen
      restart(option);
}


No, not at all. It's just the way he explains things when lecturing. I've learned more from YouTube videos and other tutorial more than I have from him in the past few days. I'm still very new to programming.

And I'll do that right now, thanks.
Topic archived. No new replies allowed.