HELP! What is wrong with this code?

[N1ghtm4r3]

I am trying to accomplish a few tasks with this program:

#include "stdafx.h"
#include <iostream>
#include <istream>
#include <string>
#include <algorithm>


using namespace std;

// Declaring my Arrays
string player_names[10];                                
int player_scores[10];      
int x,i,j,n,temp;

void SortScores(int player_scores[], int temp);


int main()

#define SCORE_NUM (sizeof(player_scores)/sizeof(int))

//This section gets the first 10 players names.
{                                                                              // Line 30 - Missing bracket?
for (int x=0; x<=9; x++) {                             
cout << "\nEnter name of Player " << x+1 << ":\n" ;      
cin >> player_names[x+1]; 
}
}

//This section gets the 10 players scores.
for (int x=0; x<=9; x++) {                            
cout << "\nEnter score of Player " << x+1 << ":\n" ;     
cin >> player_scores[x+1];                               
}

//This is declaring more variables.
int max = player_scores[0];                            
int max_index = 0;                                     

//This section is to calculate who had the highest score.
for (int x=0; x<=9; x++) {                              
if (player_scores[x+1] > max) {                         
max = player_scores[x+1];                               
max_index = x+1;                                         
}
}

void SortScores(int player_scores[], int temp) //This was my attempt at an exchange sort
{
	if (temp < 2)
		return;
	for (int i=0; i < temp-1; i++)
	{
		for (int j=i+1; j < temp; j++)
		{
			if (player_scores[i] < player_scores[j])
			{
				int n = player_scores[i];
				player_scores[i] = player_scores[j];
				player_scores[j] = n;
			}
		}
	}
}

SortScores(player_scores, SCORE_NUM); //This was an attempt to output my results

for (unsigned int i=0; i < SCORE_NUM; i++){

	cout << player_scores[i] << " ";
}
cout << endl;

system ("pause");                                      
return 0;                                              
} 

I'm kind of winging it now, making adjustments from reference. I am trying to sort the Players Scores so that I can output them from highest to lowest.

I'm working from this source to make the adjustment:

http://www.toptip.ca/2010/04/c-exercises-exchange-sort.html

Then I need to sort the Players names alphabetically, but I haven't even started with that section of code yet.

Thanks for any help, suggestions, opinions.

**EDIT**

When I try to compile, I only get two errors:

1>\visual studio 2010\projects\improving your code\improving your code\improving your code.cpp(58): error C2601: 'SortScores' : local function definitions are illegal

1>\visual studio 2010\projects\improving your code\improving your code\improving your code.cpp(30): this line contains a '{' which has not yet been matched

[Computergeek01]

OPINIONS:
- player_names and player_score should be data members of an object.

- The macro you define at Line 20 should be a function.

- You are not including cstdlib.h so you should not be using the "system(...)" function; which you would not need if you define an object with a destructor.

- "void SortScores(...)" is trying to return a '0'. Stop that.

- Your indentations are sporatic, you had a good system to start but then got sloppy, this makes your code hard to read.

EDIT: Because you now have posted errors.

- "SortScores(...)" is expecting an array for the first argument, but on Line 66 you try to pass it a single 'int'. Also, you might be burying your self here, rethink this recursive call.

- You appear to have an orphan "for(...)" loop on Line 31 and Line 41. But again, your indentations broke down and are now hard to read.

[N1ghtm4r3]

Well, I should mention that I am fairly new to all this. I'm picking it up quick, in my opinion, but I am still trying to understand some concepts.

Thanks for all the suggestions, though I'm not sure how to fix most of them. I'm going to try to clean it up a bit and I'll repost the results in a minute.

Thanks a lot!!!

--How do I make this macro a function?

--How do I stop SortScores from returning 0?

--What do you mean by orphan "for(...)" loop?

[mcqueen]

Also you should say

int x ; //declare variable outside of loop/loop parentheses.
for (x=0; x<9; x++)
{

[N1ghtm4r3]

Cleaned it up a bit:

// Week 2 Assignment.cpp : Defines the entry point for the console application.
//
//David
//Programming I - Week 2
//

//Classes I am using
#include "stdafx.h"
#include <iostream>
#include <istream>
#include <string>
#include <algorithm>
#include <cstdlib>

#define SCORE_NUM (sizeof(player_scores)/sizeof(int))

using namespace std;

void SortScores(int player_scores[], int temp);

// Declaring my Arrays
string player_names[10];                                
int player_scores[10];      
int x,i,j,n,temp;

int main()

{
//This section gets the first 10 players names.
for (int x=0; x<=9; x++) {                             
cout << "\nEnter name of Player " << x+1 << ":\n" ;      
cin >> player_names[x+1]; 
}


//This section gets the 10 players scores.
for (int x=0; x<=9; x++) {                            
cout << "\nEnter score of Player " << x+1 << ":\n" ;     
cin >> player_scores[x+1];                               
}

//This is declaring more variables.
int max = player_scores[0];                            
int max_index = 0;                                     

//This section is to calculate who had the highest score.
for (int x=0; x<=9; x++) {                              
if (player_scores[x+1] > max) {                         
max = player_scores[x+1];                               
max_index = x+1;                                         
}
}

void SortScores(int player_scores[], int temp);{
	if (temp < 2)
		return 0;
	for (int i=0; i < temp-1; i++){
		for (int j=i+1; j < temp; j++){
			if (player_scores[i] < player_scores[j])
			{
				int n = player_scores[i];
				player_scores[i] = player_scores[j];
				player_scores[j] = n;
			}
		}
	}
}

SortScores(player_scores[], SCORE_NUM)

{
for (unsigned int i=0; i < SCORE_NUM; i++){
	cout << player_scores[i] << " ";
}
cout << endl;
}

system ("pause");                                      
return 0;                                              
} 

[N1ghtm4r3]

@mcqueen

I'm not exactly sure what you mean by this. I declared x as an int along with i,j,n, and temp.

Care to clarify?

[mcqueen]

Ah i see, then you don't need to put int in the for loop parentheses just put x=0; x<9; x++ thats all

[Computergeek01]

I'd say don't make "x" a global variable at all.

Think, can you combine your "Enter Score..." and "Enter Player Name..." for loops?

Now that I read what that Macro is doing, you can just eliminate it. Since you're using it to designate the number of players in your global array, which is constant, why not just define a constant?

"void SortScores(...)" is still trying to return an integer (Line 79), please address that.

There is SO much that could be optimized out of this, I guarentee there is someone right now rewritting this in their head.

[N1ghtm4r3]

Well, I can't wait until the day I can do that. But for now, I'm stuck trying to learn all there is to know. Thanks for all your help and suggestions though, they are helping me out.

--Why not make "x" global? Is there a reason?

--I probably could combine the FOR loops that you refer to, but again, I'm new at this. I was trying to make it work and it did, do you care to give me a hint on how to combine them?

--I just added the macro because it was in the reference I was looking at. I guess I didn't even know what it was for other than they did it in the reference so I figured I would do it to. Would you mind clarifying how I can change it as you suggested, to a constant?

--I'm not sure how to address Line 79, what should it be returning rather than an integer? When I try to make the int "player_scores" into the array form, it gives me errors like I can't.

Thanks for everything. Just trying to understand this all. It's overwhelming at times.

[N1ghtm4r3]

Would this do, to combine the loops?

for (int x=0; x<=9; x++) {                             
cout << "\nEnter name of Player " << x+1 << ":\n" ;      
cin >> player_names[x+1]; 
cout << "\nEnter score of Player " << x+1 << ":\n" ;     
cin >> player_scores[x+1]; 
}

**EDIT**

Actually, I'm sure it won't. I just thought about it. Any help would be appreciated.

[Computergeek01]

You're close, what's with the "x+1" stuff?

[N1ghtm4r3]

Thats so that it display "Player 1" rather than "Player 0". You know, because arrays initialize at 0.

[Computergeek01]

Yeah but you have the option of initializing "x" as 1...

[N1ghtm4r3]

Oh, hmmm. I guess I hadn't thought of it like that. This is true.

Remember, beginner here. lol.

But thanks a lot for clarifying that, it's nice to see it from another perspective.

[N1ghtm4r3]

Ok, so I have revised it to how you mentioned.

Now, any tips on how to combine these two loops?

for (int x=1; x<=10; x++) {                             
cout << "\nEnter name of Player " << x << ":\n" ;      
cin >> player_names[x]; 
cout << "\nEnter score of Player " << x << ":\n" ;     
cin >> player_scores[x]; 
}

[Computergeek01]

What do you mean? You just did combine them.

[N1ghtm4r3]

oh, alright. So that will work properly then? I can't test unless I run it by itself. I'm still trying to work out this sorting and displaying of the scores from highest to lowest.

I'm lost...

**EDIT**

Alright, I ran it by itself and confirms it works like that. Now to work on my original problem.
Thanks a lot Computergeek, you've been a great help so far. Keep it coming...

[Computergeek01]

No worries, it actually took me more time then I will ever admit to realize that you can speed up trouble shooting by commenting out sections of code to isolate others ("//"). Give it a try, it beats the hell out of starting a new project to test one block of code or, even worse, deleting lines that you know you'll be rewritting later.

[N1ghtm4r3]

Alright, tell me what is going on here please. I feel like it's false hope I'm only getting one error message. But I have no clue what it is. Am I doing something Bass Ackwards here?

#include "stdafx.h"
#include <iostream>
#include <istream>
#include <string>
#include <algorithm>
#include <cstdlib>

#define SCORE_NUM (sizeof(player_scores)/sizeof(int))

using namespace std;

void SortScores(int player_scores[], int temp);

// Declaring my Arrays
string player_names[10];                                
int player_scores[10];      
int x,i,j,n,temp;

int main()

{
//This section gets the first 10 players names.
for (int x=1; x<=10; x++) {                             
cout << "\nEnter name of Player " << x << ":\n" ;      
cin >> player_names[x]; 
cout << "\nEnter score of Player " << x << ":\n" ;     
cin >> player_scores[x]; 
}

//This is declaring more variables.
int max = player_scores[0];                            
int max_index = 0;                                     

//This section is to calculate who had the highest score.
for (int x=1; x<=10; x++) {                              
if (player_scores[x] > max) {                         
max = player_scores[x];                               
max_index = x;                                         
}
}

void SortScores(int player_scores[], int temp);

	for (int i=0; i < temp-1; i++){
		for (int j=i+1; j < temp; j++){
			if (player_scores[i] < player_scores[j])
			{
				int n = player_scores[i];
				player_scores[i] = player_scores[j];
				player_scores[j] = n;
			}
		}
	}


SortScores(player_scores, SCORE_NUM);

{
for (unsigned int i=0; i < SCORE_NUM; i++){
	cout << player_scores[i] << " ";
}
cout << endl;
}

system ("pause");                                      
return 0;                                              
}

mproving Your Code.obj : error LNK2001: unresolved external symbol "void __cdecl SortScores(int * const,int)" (?SortScores@@YAXQAHH@Z)
1>c:\users\nightmare\documents\visual studio 2010\Projects\Improving Your Code\Debug\Improving Your Code.exe : fatal error LNK1120: 1 unresolved externals

[Computergeek01]

If you don't fix Line 66, I'm not going to help you anymore. I know I've gone off on a bit of a tangent but I've also mentioned this twice so far.