1st Program Critique

Hello all,

I am new to CPP programming. I have some PHP experience but have finally taken the plunge to desktop programming. I've begun in the world of C / C++. I have created my first program and would like you to take a look at it and tell me if I have made any glaring mistakes or have any other bad habits that should be checked now.

The application is a simple music scale generator. It accepts input from the user and calculates the notes in various scales based on the root note they entered.
So here ya go,

GenerateScale.cpp
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
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
/**  This first programming test is a simple Music scale generator
 **	 User input is accepted (not very well) and it spits out the scale based on the 
 **  Root note they entered
 **
 **  author Nathan Chronister
 **/
 
#include <iostream>
#include <string>
#include <cstdlib>

using namespace std;

/** Declare the scale steps for a few scales to use in calculations later 
 **/ 
	int stepsArray[5][7] = {
		{2,2,1,2,2,2,1}, // major scale steps
		{2,1,2,2,1,2,2}, // natural Minor scale steps
		{2,1,2,2,1,3,1 }, // harmonic minor scale steps
		{2,1,2,2,2,2,1}, // melodic minor scale steps
		{2,2,1,2,2,1,2} // mixolydian scale steps
	};

/** Declare the steps names as labels for the above arrays
 ** 
 **/ 
	string stepsNames[5] = {"Major", "Natural Minor", "Harmonic Minor", "Melodic Minor","Mixolydian"};
 
 /** Declare the row / col of the root position item
  **
  **/
  int rootPosition[2];
  
/** Declare the scale positions with common flats and sharps 
 **/ 	
	string scalePositions[3][12] = {
		{"A","","B","C","","D","","E","F","","G",""}, // Chromatic positions / Diatonic Notes
		{"","Bb","Cb","","Db","","Eb","Fb","","Gb","","Ab"}, // Flat positions
		{"","A#","","B#","C#","","D#","","E#","F#","","G#"} // Sharp Positions
	};

/** Declare var noteMap to hold the end array we need to display
 **/ 
	int scaleMap[5][8];
	
/** This will be used to display the scaleMap array
 **/ 
 void displayMap(); 
	
/** Declare method we will use to get input from the user and return it 
 **/
	string getInput();
	
/** Declare method we will use to take user input and return code or the 
 **  index position of the root note 
 **/
	int findIndex(string input);

/** Takes the root index number and the raw string and returns the scale map and displays it
 **/  
	int getScaleMap(int rootIndex, string input);
	
/** Declare our banner method
 **/
	void displayBanner(void);
	
/**
 **  Begin our main method
 **/
	int main(void){
		
	/*
	 * Display the banner for this program with directions to the user 
	 */
		displayBanner();
			
	/*
	 * Call Get Input function to get the raw input from the user
	 */
		string input = getInput();
		
	/*
	 * Get the root note via the findIndex function
	 */
		int root = findIndex(input);
		
	/*
	 * determine what to do with input recieved after it has been parsed
	 */
		switch(root){
			
			/*
			 *  The user passed a Q so code 998 signals a break which jumps to end of main
			 */
			case 998:
				cout << "Goodbye!!";
			break;
		
			/*
			 *  Unknown input, nothing matched our arrays nor was it a quit signal. Output a msg to 
			 *  user and start over.
			 */
			case 999:\
				cout << "Invalid Input, try again." << endl;
				main();
			break;
			
			/*
			 * It found a match in the arrays, get the scale map and print on screen
			 */
			default:
				system("cls");
				getScaleMap(rootPosition[1], input);	
					
		}
	}

/**
 **  getScaleMaps takes the root note index number returned by findIndex and the raw input 
 **  The raw input is for display as a label in brackets. This is poorly implemented and needs 
 **  to be abstracted out so that it displays not just the major scale, but any scales that are  
 **  defined
 **/
	int getScaleMap(int rootIndex, string input){
			
			// Loop through each row of the stepsArray 
			for(int row = 0; row < sizeof stepsArray / sizeof stepsArray[0]; row++){
				
				// The 0 element in each row will be the rootIndex 
				scaleMap[row][0] = rootPosition[1];
				
				// Loop through each column of the stepsArray
				for(int col = 0; col < sizeof stepsArray[0] / sizeof stepsArray[0][0]; col++){		
					
					// Add the scaleMap and stepsArray elements together to get the "map"
					// that the scales will need followed
					int stepCount = scaleMap[row][col] + stepsArray[row][col];
					
					// if our step counter is greater than 11, then subtract 12
					stepCount = ( stepCount > 11) ? stepCount - 12 : stepCount;
					
					//set the next scaleMap column to the step count var
					scaleMap[row][col+1] = stepCount;
				} 

			}
		// call our display map function now
		displayMap();
		// go back to main
		main();		
	}
/** Prompt the user to enter the scale they want    
 **/
	string getInput(){ 

		// declare vars to use in this method
			string usrInput; 
			
		// ask user which root note
			cout << "Enter Note: ";
			cin >> usrInput; 
		return usrInput;
	}

/**This method analyzes the input entered by the user.  It returns 998 if Q was entered.
 ** It then loops through the scalePositions multi dimensional array and returns the col number
 ** of the match it finds, otherwise the loop ends and it returns a 999 signaling invalid input
 **/
	int findIndex(string input){
		
		// if Q was passed in upper or lower case, return 998
			if(input == "Q" || input == "q"){ return 998; }
		
			// Loop through each row of the scale positions array
			for(int row = 0; row < 3; row++){
				
				// Loop through each column of the scale postions array
				for(int col = 0; col < 12; col++){
					
					// Compare input value to the row/col we are on
					if(input == scalePositions[row][col]){
						// return the row / col position if it matches
						rootPosition[0] = row;
						rootPosition[1] = col;
						return col;
					}
				}
			} 	
			
			// Not Q/q, nor did it match so return 999 for invalid input
				return 999;
			
	}

/** Display a simple banner at the top of the program with "title" and instructions
 **  
 **/
	void displayBanner(void){
	
	  cout << " ************************************************************************* " << endl;
	  cout << " *                      Music Scale Generator                            * " << endl;
	  cout << " *                                                                       * " << endl;
	  cout << " *         Enter the root Note you want to see the scales for            * " << endl;
	  cout << " *        Note must be uppercase, enter # for sharp and b for flat)      * " << endl;
	  cout << " *                            Enter Q for quit?                          * " << endl;
	  cout << " ************************************************************************* " << endl;
	  cout << endl << endl;
}

void displayMap(){ 

		// loop through the rows of the scaleMap array
		for(int row = 0; row < 5; row++){
			
				// output the name of the scale using the stepsNames array
				cout << endl << stepsNames[row] << " Scale: " << endl;
				
			// loop through the cols of the scaleMap array
			for(int col = 0; col < 8; col++){
				
				// check row 0 for this position
				if(scalePositions[0][scaleMap[row][col]] != ""){
					
					// if Row 0, col COL of scalePositions is not blank output the note (natural notes)
					cout << scalePositions[0][scaleMap[row][col]] <<  " ";
					
					// else, check row 2 for this position
				}else if(scalePositions[2][scaleMap[row][col]] != ""){
					
					// If row 2, col COL of scalePositions is not blank, output the note (sharps)
					cout << scalePositions[2][scaleMap[row][col]] <<  " ";
					
					
					// check row 1  for this position (flats)
				}else if(scalePositions[1][scaleMap[row][col]] != ""){
					
					// If row 1, col COL of scale positions is not blank , output the note 
					cout << scalePositions[1][scaleMap[row][col]] <<  " ";
				}
				  
	  
			}
			cout << endl;
		}
		cout << endl;
} 


Thank you for your input. It is appreciated.
Last edited on
Two things.

First, putting comments before every line of code just makes everything more difficult to follow. If you choose meaningful and descriptive names for your objects and functions, the intent is conveyed as part of the code structure. Another problem with comments is that they tend to go out of sync with the code as it changes.

Second, calling main() is illegal. Find a different way to start over from the start.
For a first program its pretty nice.
Learn to define constants at the top. Unlikely to change here, but if you have
int array [size];
...
more code
for (... size ...)
... more code
size again
and again

its better if you can just change size in one place rather than try to figure out if this 5 needs to change but that 5 does not if you change the code around.

one line comments are //comments to end of this line ...

As noted, always beware unintentional recursion (which is a function that calls itself!). This is a while loop, in your code. Recursion is powerful, but a topic for another day.



Topic archived. No new replies allowed.