Filling and sorting an array

[madams8377]

Writing an application that accepts 10 numbers and displays them in descending order. It almost works but it's outputting one junk number and 9 of my 10 input numbers. What am I missing, please?

  #include <iostream>
using namespace std;
int main()
{
	const int SIZE = 10;
	int NUMBERS[SIZE];
	int i;
	int j;
	int temp;
		
	for (i = 0; i < SIZE; i++)
	{
		cout << "Please enter a number: ";
		cin >> NUMBERS[i];
	}//endfor
	
	for(i=0; i < SIZE; i++)
		for(j = 0; j < SIZE; j++)
	{
		if (NUMBERS[j] < NUMBERS[j+1])
			{
				temp = NUMBERS[j];
				NUMBERS[j] = NUMBERS[j+1];
				NUMBERS[j+1] = temp;
			}//endif
	}//endfor
	
	cout << "Sorted list" << endl;
	cout << "===========" << endl;
	for (i = 0; i < SIZE; i++)
	{
		cout << "Number " << i+1 << ": " << NUMBERS[i] << endl;
	}
	//endfor
}

[Repeater]

NUMBERS[10] does not exist.

Your code uses NUMBERS[10].

[madams8377]

I don't understand what you mean. The array does exist and it accepts and sorts 9 of the 10 integers that are input. It's just one number that's wrong.

[Repeater]

NUMBERS[0] exists.
NUMBERS[1] exists.
NUMBERS[2] exists.
NUMBERS[3] exists.
NUMBERS[4] exists.
NUMBERS[5] exists.
NUMBERS[6] exists.
NUMBERS[7] exists.
NUMBERS[8] exists.
NUMBERS[9] exists.

NUMBERS[10] does not exist.

[nuderobmonkey]

What Repeater means is that you use NUMBERS[j+1] in your code, and when j will be 9, then you access NUMBERS[10] which does not exist.

[Inspireftw]

If he starts his loop at 0, it should never hit the 10th element of the array. I think your problem is that you have a constant int for the size of the array. Since your program doesn't ask the user for the size of the array, why would you want to use a constant integer?

I rewrote it a bit and this seems to work.

#include <iostream>

using namespace std;

int main()
{
	int myArray[10];
	int temp;

	cout << "Please enter a number: ";

	for (int i = 0; i < 10; i++)
	{
		cin >> myArray[i];
	}

	for (int i = 0; i < 10; i++)
		for (int j = 0; j < 10; j++)
		{
			if (myArray[j] < myArray[j + 1])
			{
				temp = myArray[j];
				myArray[j] = myArray[j + 1];
				myArray[j + 1] = temp;
			}
		}

	cout << "Sorted list" << endl;
	cout << "===========" << endl;
	for (int i = 0; i < 10; i++)
	{
		cout << "Number " << i + 1 << ": " << myArray[i] << endl;
	}
	return 0;
}

[lastchance]

@Inspireftw,
Please READ what @Repeater and @Nuderobmonkey wrote. You have just negated all that they said. YOU are still accessing beyond the end of the array (on your lines 23 and 24, when j = 9).

I think your problem is that you have a constant int for the size of the array.

That is also nonsense. @madams8377 correctly used a compile-time constant, which only needs to be adjusted ONCE if the size is changed. You, on the other hand, have just used the "magic number" 10, which has to be changed consistently on FIVE separate lines should the array size change.


Please do not make the OP's problem worse.

[Inspireftw]

@lastchance
The program will never execute 23 and 24 when the array is in the 9th element. If J = 9 then the if statement is false...

[lastchance]

Inspireftw wrote:

If J = 9 then the if statement is false...

If j = 9 then the if statement says
if (myArray[9] < myArray[10])
How do you even propose to evaluate that? It's neither "true" nor "false", it is "undefined" and you will be quite lucky if it doesn't segfault.

myArray[10] DOES NOT EXIST. See @Repeater's post at
http://www.cplusplus.com/forum/beginner/244972/#msg1084165

[madams8377]

I understand the logic now, and I think I fixed it but let me know...

#include <iostream>
using namespace std;
int main()
{
	const int SIZE = 10;
	int NUMBERS[SIZE];
	int i;
	int j;
	int temp;
		
	for (i = 0; i < SIZE; i++)
	{
		cout << "Please enter a number: ";
		cin >> NUMBERS[i];
	}//endfor
	
	for(i=0; i < SIZE; i++)
		for(j = 0; j < SIZE-1; j++)
	{
		if (NUMBERS[j] < NUMBERS[j+1])
			{
				temp = NUMBERS[j];
				NUMBERS[j] = NUMBERS[j+1];
				NUMBERS[j+1] = temp;
			}//endif
	}//endfor
	
	cout << "Sorted list" << endl;
	cout << "===========" << endl;
	for (i = 0; i < SIZE; i++)
	{
		cout << "Number " << i+1 << ": " << NUMBERS[i] << endl;
	}
	//endfor
}

[Ganado]

lastchance wrote:

and you will be quite lucky if it doesn't segfault.

*unlucky :)

Also... I agree with MikeyBoy, that guy you replied to is a troll...

madams8377,
Looks OK, but note that you should try to keep variables to the narrowest scope in which they are needed.
Your i and j variables can be written in the for loops themselves to reduce their scope.
for (int i = 0; i < SIZE; i++)

Your temp variable can be moved to be declared right where it's used.

int temp = NUMBERS[j];
NUMBERS[j] = NUMBERS[j+1];
NUMBERS[j+1] = temp;

Also, ALLCAPS name are usually meant to be macros or, in some styles, constants. Your NUMBERS array is not a constant or macro, so I would avoid using all-caps.