What is wrong with my sorting function?

[musique]

What is wrong with my sorting function? I am trying to put numbers in ascending numbers.

void sort(int array[], int numts)
{
	int lowIndex, lowest;

	for (int count = 0; count < (numts-1); count++)
	{
		lowIndex = count;
		lowest = array[count];
		
		for (int index = count + 1; index < numts; index++)
		{
			if (array[index] < lowest)
			{
				lowest = array[index];
				lowIndex = index;
			}
		}
		array[lowIndex] = array[count];
		array[count] = lowest;


	}
	return;
}

[VernonDozier]

What is wrong with my sorting function? I am trying to put numbers in ascending numbers.

void sort(int array[], int numts)
{
	int lowIndex, lowest;

	for (int count = 0; count < (numts-1); count++)
	{
		lowIndex = count;
		lowest = array[count];
		
		for (int index = count + 1; index < numts; index++)
		{
			if (array[index] < lowest)
			{
				lowest = array[index];
				lowIndex = index;
			}
		}
		array[lowIndex] = array[count];
		array[count] = lowest;


	}
	return;
}

Why do you say there's something wrong with it? It ran fine for me.

[ArkM]

>What is wrong with my sorting function?
Probably it works. Try to test it ;)
What's your problem?

[StuXYZ]

What is wrong with my sorting function? I am trying to put numbers in ascending numbers.

Nothing, in the fact that it actually works, BUT it is a bubble sort
and hence very inefficient.

If you are still having problems check your calling routine, are you
using an array followed by a count. Is that count correct?
e.g. I used this to test it.

int main()
{
  srand(time(0));
  int a[10];
  for(int i=0;i<10;i++)
    a[i]=rand() % 100;
  sort(a,10);
  for(int i=0;i<9;i++)
    {
      if (a[i+1]<a[i])
        {
      std::cout<<"Probem at i == "<<i<<std::endl;
      for(int i=0;i<10;i++)
        std::cout<<"A == "<<a[i]<<std::endl;
      return -1;
    }
    }
  return 0;
}

[ArkM]

>BUT it is a bubble sort...
It's a selection sort...
>and hence very inefficient.
That's right (BUT for large arrays only) ;)

[vmanes]

Nothing, in the fact that it actually works, BUT it is a bubble sort and hence very inefficient.

Nope, it's selection sort. Which is generally a bit more efficient than bubble.

And as we discussed several days back, whether any of the n-squared sorts are really inefficient is a matter of size of the data being sorted. On current hardware, data of up to a few thousand items, you won't notice the time it takes.

[musique]

Why do you say there's something wrong with it? It ran fine for me.

Really? Cause it didn't sort the numbers...let me show all the code then...maybe it is just not displaying....

/***This program dynamically allocates an array large enough to
hold a user-defined number of test scores***/
#include <iostream>
#include <iomanip>
using namespace std;

//Function prototypes
int getnumtestscores();
void gettestscores(int[], int);
void showArray(int [], int);
void sort(int [], int);
double average(int[], int);


int main()
{
	int numts = getnumtestscores();

	int *testscores = new int[numts];		//To dynamically allocate an array

	gettestscores(testscores, numts);

	cout << "The testscores are: \n"; showArray (testscores, numts);

	cout << "The testscores sorted are: "; sort (testscores, numts);

    cout << "The average of all " << numts << " test scores is: " 
         << fixed << showpoint << setprecision(2)
         << average(testscores, numts);
	cout << endl;

    // Free dynamically allocated array in memory
    delete [] testscores;
    // Make scores point to null
    testscores = 0;

	return 0;
}

int getnumtestscores()
{
    int num;

    cout << "\n How many test scores do you want to enter : ";
    for(;;) // loop forever  ... until return
    { 
        cin >> num;
     
        return num;
    }  
}

void gettestscores(int testscores[], int numts)
{
	cout << "What are the testscores for the students?\n";

	for (int count = 0; count < numts; count++)
	{
		cout << "Testscore " << setw(2) << (count+1) << ":";

		if (testscores < 0)		//Input validation
		{
			cout << "An invalid score was entered, please enter a score more than 0\n";
		}

		cin >> testscores[count];
	}
}

void sort(int array[], int numts)
{
	int lowIndex, lowest;

	for (int count = 0; count < (numts-1); count++)
	{
		lowIndex = count;
		lowest = array[count];
		
		for (int index = count + 1; index < numts; index++)
		{
			if (array[index] < lowest)
			{
				lowest = array[index];
				lowIndex = index;
			}
		}
		array[lowIndex] = array[count];
		array[count] = lowest;


	}

	return;
}

void showArray(int array[], int numts)
{
	for (int count = 0; count < numts; count++)
		cout << array[count] <<" \n";
	cout << endl;
}

double average(int array[], int numts)
{
	int total = 0;

	for (int count = 0; count < numts; count++)
	
		total +=array[count];

	  return total/numts;
}

[musique]

>BUT it is a bubble sort...
It's a selection sort...
>and hence very inefficient.
That's right (BUT for large arrays only) ;)

Selection sort is the only one I know how to use but I posted all the code this time, I am thinking maybe it is not being displayed properly, what do you think Vernon?

[Sky Diploma]

Really? Cause it didn't sort the numbers...let me show all the code then...maybe it is just not displaying....

/***This program dynamically allocates an array large enough to
hold a user-defined number of test scores***/
#include <iostream>
#include <iomanip>
using namespace std;

//Function prototypes
int getnumtestscores();
void gettestscores(int[], int);
void showArray(int [], int);
void sort(int [], int);
double average(int[], int);


int main()
{
	int numts = getnumtestscores();

	int *testscores = new int[numts];		//To dynamically allocate an array

	gettestscores(testscores, numts);

	cout << "The testscores are: \n"; showArray (testscores, numts);

	cout << "The testscores sorted are: "; sort (testscores, numts);

    cout << "The average of all " << numts << " test scores is: " 
         << fixed << showpoint << setprecision(2)
         << average(testscores, numts);
	cout << endl;

    // Free dynamically allocated array in memory
    delete [] testscores;
    // Make scores point to null
    testscores = 0;

	return 0;
}

int getnumtestscores()
{
    int num;

    cout << "\n How many test scores do you want to enter : ";
    for(;;) // loop forever  ... until return
    { 
        cin >> num;
     
        return num;
    }  
}

void gettestscores(int testscores[], int numts)
{
	cout << "What are the testscores for the students?\n";

	for (int count = 0; count < numts; count++)
	{
		cout << "Testscore " << setw(2) << (count+1) << ":";

		if (testscores < 0)		//Input validation
		{
			cout << "An invalid score was entered, please enter a score more than 0\n";
		}

		cin >> testscores[count];
	}
}

void sort(int array[], int numts)
{
	int lowIndex, lowest;

	for (int count = 0; count < (numts-1); count++)
	{
		lowIndex = count;
		lowest = array[count];
		
		for (int index = count + 1; index < numts; index++)
		{
			if (array[index] < lowest)
			{
				lowest = array[index];
				lowIndex = index;
			}
		}
		array[lowIndex] = array[count];
		array[count] = lowest;


	}

	return;
}

void showArray(int array[], int numts)
{
	for (int count = 0; count < numts; count++)
		cout << array[count] <<" \n";
	cout << endl;
}

double average(int array[], int numts)
{
	int total = 0;

	for (int count = 0; count < numts; count++)
	
		total +=array[count];

	  return total/numts;
}

After Sorting You arent using your display function to look at the sorted variable.

int main()
{
	int numts = getnumtestscores();

	int *testscores = new int[numts];		//To dynamically allocate an array

	gettestscores(testscores, numts);

	cout << "The testscores are: \n"; showArray (testscores, numts);

	cout << "The testscores sorted are: "; sort (testscores, numts); //No Display After Sort.

    cout << "The average of all " << numts << " test scores is: " 
         << fixed << showpoint << setprecision(2)
         << average(testscores, numts);
	cout << endl;

    // Free dynamically allocated array in memory
    delete [] testscores;
    // Make scores point to null
    testscores = 0;

	return 0;
}

Check Out Line 11 in the above code. There you dont display the sorted list again.
So you should again call showArray (testscores, numts); Like this

int main()
{
	int numts = getnumtestscores();

	int *testscores = new int[numts];		//To dynamically allocate an array

	gettestscores(testscores, numts);

	cout << "The testscores are: \n"; showArray (testscores, numts);

	cout << "The testscores sorted are: \n\n"; sort (testscores, numts); //Display Function Here.
showArray (testscores, numts);
    cout << "The average of all " << numts << " test scores is: " 
         << fixed << showpoint << setprecision(2)
         << average(testscores, numts);
	cout << endl;

    // Free dynamically allocated array in memory
    delete [] testscores;
    // Make scores point to null
    testscores = 0;

	return 0;

[musique]

Thank you skydiploma, it works! Sigh! So simple...all I have to do now is input pointers in it...everytime I place a pointer in the function declaration and function definition, it doesn't show errors but when it then puts an error for the function call, (error C2664: 'showArray' : cannot convert parameter 1 from 'int *' to 'int *[]')....example:

/***This program dynamically allocates an array large enough to
hold a user-defined number of test scores***/
#include <iostream>
#include <iomanip>
using namespace std;

//Function prototypes
int getnumtestscores();
void gettestscores(int[], int);
void showArray(int [], int);
void sort(int *[], int);
double average(int[], int);


int main()
{
	int numts = getnumtestscores();

	int *testscores = new int[numts];		//To dynamically allocate an array

	gettestscores(testscores, numts);

	cout << "The testscores are: \n"; showArray (testscores, numts);

	cout << "The testscores sorted are: \n"; sort (testscores, numts);

	showArray (testscores, numts);

    cout << "The average of all " << numts << " test scores is: " 
         << fixed << showpoint << setprecision(2)
         << average(testscores, numts);
	cout << endl;

    // Free dynamically allocated array in memory
    delete [] testscores;
    // Make scores point to null
    testscores = 0;

	return 0;
}

int getnumtestscores()
{
    int num;

    cout << "\n How many test scores do you want to enter : ";
    for(;;) // loop forever  ... until return
    { 
        cin >> num;
     
        return num;
    }  
}

void gettestscores(int testscores[], int numts)
{
	cout << "What are the testscores for the students?\n";

	for (int count = 0; count < numts; count++)
	{
		cout << "Testscore " << setw(2) << (count+1) << ":";

		if (testscores < 0)		//Input validation
		{
			cout << "An invalid score was entered, please enter a score more than 0\n";
		}

		cin >> testscores[count];
	}
}

void sort(int array*[], int numts)
{
	int lowIndex, *lowest;

	for (int count = 0; count < (numts-1); count++)
	{
		lowIndex = count;
		lowest = array[count];
		
		for (int index = count + 1; index < numts; index++)
		{
			if (*(array[index]) < *lowest)
			{
				lowest = array[index];
				lowIndex = index;
			}
		}
		array[lowIndex] = array[count];
		array[count] = lowest;


	}

	return;
}

void showArray(int array[], int numts)
{
	for (int count = 0; count < numts; count++)
		cout << array[count] <<" \n";
	cout << endl;
}

double average(int array[], int numts)
{
	int total = 0;

	for (int count = 0; count < numts; count++)
	
		total +=array[count];

	  return total/numts;
}

[mvmalderen]

Want an example of a bubble sort? Check out this :)

[musique]

Thanks tux, do you have any examples of pointers being used in functions? My pointers are screwing up my program...

[Sky Diploma]

>>void sort(int *[], int);
That Should Actually be

void sort(int *,int);

You can learn some good stuff about pointers here

POINTER Explique ;)

[mvmalderen]

Thanks tux, do you have any examples of pointers being used in functions? My pointers are screwing up my program...

You actually don't need to use pointers for that purpose, just pass the array as an argument to the function (it's just as efficient as using pointers, because an array's name is treated as a constant pointer (a pointer which memory address is constant, which means it cannot be changed) to the first element of the array, you can use the index operator to get an element from the array)

:)

Edit:: Actually I try to avoid pointers as much as possible, only when there's no other way left I will use it ...
Edit:: BTW, the bubble sort example where I've posted the link in my previous post did use a pointer...

[mvmalderen]

Want some further info on pointers?

• http://www.augustcouncil.com/~tgibson/tutorial/ptr.html
• http://daweidesigns.netfirms.com/cgi-bin/pointers.php
• http://www.codersource.net/c++_pointers.html
• http://www.cplusplus.com/doc/tutorial/pointers/
• http://home.netcom.com/~tjensen/ptr/pointers.htm

Hope this helps !

[musique]

>>void sort(int *[], int);
That Should Actually be

void sort(int *,int);

You can learn some good stuff about pointers here

POINTER Explique ;)

I tried that but it didn't work. When I change it back, it gives me an error with this function call...

sort (testscores, numts);

[VernonDozier]

I tried that but it didn't work. When I change it back, it gives me an error with this function call...

sort (testscores, numts);

Post the updated code. We don't know what you have changed. It's very unlikely that you want this:

void sort(int *[], int);

Most likely you want this:

void sort(int [], int);

or this:

void sort(int *, int);

Since I guess you have decided to use pointers, I'm guessing it's the latter, as Sky Diploma suggested. Probably the function and function call don't match each other, so post the updated code.

[mvmalderen]

What do you mean with it doesn't work?

• Is it compiling or not?
• Or are you getting unexpected output?

[musique]

Post the updated code. We don't know what you have changed. It's very unlikely that you want this:

void sort(int *[], int);

Most likely you want this:

void sort(int [], int);

or this:

void sort(int *, int);

Since I guess you have decided to use pointers, I'm guessing it's the latter, as Sky Diploma suggested. Probably the function and function call don't match each other, so post the updated code.

/***This program dynamically allocates an array large enough to
hold a user-defined number of test scores***/
#include <iostream>
#include <iomanip>
using namespace std;

//Function prototypes
int getnumtestscores();
void gettestscores(int[], int);
void showArray(int [], int);
void sort(int *[], int);
double average(int[], int);


int main()
{
	int numts = getnumtestscores();

	int *testscores = new int[numts];		//To dynamically allocate an array

	gettestscores(testscores, numts);

	cout << "The testscores are: \n"; showArray (testscores, numts);

	cout << "The testscores sorted are: \n"; sort (testscores, numts);

	showArray (testscores, numts);

    cout << "The average of all " << numts << " test scores is: " 
         << fixed << showpoint << setprecision(2)
         << average(testscores, numts);
	cout << endl;

    // Free dynamically allocated array in memory
    delete [] testscores;
    // Make scores point to null
    testscores = 0;

	return 0;
}

int getnumtestscores()
{
    int num;

    cout << "\n How many test scores do you want to enter : ";
    for(;;) // loop forever  ... until return
    { 
        cin >> num;
     
        return num;
    }  
}

void gettestscores(int testscores[], int numts)
{
	cout << "What are the testscores for the students?\n";

	for (int count = 0; count < numts; count++)
	{
		cout << "Testscore " << setw(2) << (count+1) << ":";

		if (testscores < 0)		//Input validation
		{
			cout << "An invalid score was entered, please enter a score more than 0\n";
		}

		cin >> testscores[count];
	}
}

void sort(int *array[], int numts)
{
	int lowIndex, *lowest;

	for (int count = 0; count < (numts-1); count++)
	{
		lowIndex = count;
		lowest = array[count];
		
		for (int index = count + 1; index < numts; index++)
		{
			if (*(array[index]) < *lowest)
			{
				lowest = array[index];
				lowIndex = index;
			}
		}
		array[lowIndex] = array[count];
		array[count] = lowest;
	}

	return;
}

void showArray(int array[], int numts)
{
	for (int count = 0; count < numts; count++)
		cout << array[count] <<" \n";
	cout << endl;
}

double average(int array[], int numts)
{
	int total = 0;

	for (int count = 0; count < numts; count++)
	
		total +=array[count];

	  return total/numts;
}

I tried using what tux said with void sort(int *, int); but maybe it didn't match though...

[mvmalderen]

Why did you rewrite your whole code using pointers?
Edit:: Now you've already experienced that pointers can make a mess of your program :P ...
Edit:: My suggestion is: try to avoid pointers as much as possible ...

[mvmalderen]

I've changed your function to use pointer arithmetic (the changed parts are in green-colored bold):

void sort(int [B]*array[/B], int numts)
{
	int lowIndex, *lowest;

	for (int count = 0; count < (numts-1); count++)
	{
		lowIndex = count;
		lowest = [B]array+count[/B];
		
		for (int index = count + 1; index < numts; index++)
		{
			if ([B]*(array+index)[/B] < *lowest)
			{
				lowest = [B]array+index[/B];
				lowIndex = index;
			}
		}
		[B]*(array+lowIndex)[/B] = [B]*(array+count)[/B];
		[B]*(array+count)[/B] = *lowest;
	}

	return; [I]// this return is not required, but it may be here[/I]
}

New function declaration: void sort([B]int *[/B], int); It should work now :) !!

[musique]

Why did you rewrite your whole code using pointers?
Edit:: Now you've already experienced that pointers can make a mess of your program :P ...
Edit:: My suggestion is: try to avoid pointers as much as possible ...

It didn't compile, I got this error - ch8no1.obj : error LNK2001: unresolved external symbol "void __cdecl sort(int *,int)" (?sort@@YAXPAHH@Z)

I just wanted to understand and learn pointers. The book I am using, used pointers and seemed to work fine... agreed, I would avoid using it as much as possible.

[mvmalderen]

With me it did compile, I tested it on two different compilers (Borland and MinGW)
But there's still an error in your sorting algorithm :(

Edit:: Oh I'm sorry I thought you were talking about my reply (I didn't read your post carefully enough)

[musique]

I've changed your function to use pointer arithmetic (the changed parts are in green-colored bold):

void sort(int [B]*array[/B], int numts)
{
	int lowIndex, *lowest;

	for (int count = 0; count < (numts-1); count++)
	{
		lowIndex = count;
		lowest = [B]array+count[/B];
		
		for (int index = count + 1; index < numts; index++)
		{
			if ([B]*(array+index)[/B] < *lowest)
			{
				lowest = [B]array+index[/B];
				lowIndex = index;
			}
		}
		[B]*(array+lowIndex)[/B] = [B]*(array+count)[/B];
		[B]*(array+count)[/B] = *lowest;
	}

	return;
}

New function declaration: void sort([B]int *[/B], int); It should work now :) !!

This was really good, the only problem with it is, when it sorted the numbers, the numbers were all the same as the first one entered.

[mvmalderen]

This was really good, the only problem with it is, when it sorted the numbers, the numbers were all the same as the first one entered.

Yup, that's right, I tested it, now it compiles fine we'll have to take a close look at how the algorithm works as it's not working like it should ...

[mvmalderen]

Take a look at your function getnumtestscores() :

int getnumtestscores()
{
    int num;

    cout << "\n How many test scores do you want to enter : ";
    for(;;) // loop forever  ... until return
    { 
        cin >> num;
     
        return num;
    }  
}

This red colored code can be replaced by

cin >> num;
return num;

It's doing exactly the same :) (why? when the return statement executes, the whole function execution ends)

[musique]

Take a look at your function getnumtestscores() :

int getnumtestscores()
{
    int num;

    cout << "\n How many test scores do you want to enter : ";
    for(;;) // loop forever  ... until return
    { 
        cin >> num;
     
        return num;
    }  
}

This red colored code can be replaced by

cin >> num;
return num;

It's doing exactly the same :) (why? when the return statement executes, the whole function execution ends)

Really?...I tried it and it does work, I didn't think of that...thanks...

[musique]

This was really good, the only problem with it is, when it sorted the numbers, the numbers were all the same as the first one entered.

I believe I understand how you used the pointer here instead of using the array but why when the numbers are sorted, it only shows the first number repeatedly?

[mvmalderen]

• Really?...I tried it and it does work, I didn't think of that...thanks...

  I tried it also and it did work, maybe you did something wrong? :P
  

• Just as an exercise to see whether you understand pointer arithmetic or not:
  http://www.daniweb.com/code/snippet1177.html, convert and implement this in your code (using pointer arithmetic, it's not that hard:))
  (use it as a replacement of your current sort-algorithm)
  
• It will work, 100% guaranteed :), I tested it but according to the forum rules I'm not allowed to just give the code away

[mvmalderen]

If you want exact averages you should change (in your function average ) int total = 0; to double total = 0; :P