C++ Array Sort Me

Question

Stuck on an array sorter. Have to sort numbers from largest to smallest. I'm trying two loops (one nested in the other). Here's the code:

int counter=0; // inner counter
int counter2=0; // outer counter
int sparky[14]; //array set to 14 just to simplify things
int holder; // holds the highest value
int high; //stores the position where it found the value holder

while (counter2 < howmany)
 {
  holder= sparky[counter];
  while (counter <= howmany)
   {
    counter++;
    if (sparky[counter] > holder)
     {
      holder= sparky[counter];
      high= counter; 
     }
   }
   counter2++;
   counter=counter2;
   sparky[high]= sparky[counter-1];
   sparky[counter-1]=holder;
 }

---

ARRAY UNSORTED:

data[ 0] = 9
data[ 1] = 8
data[ 2] = 7
data[ 3] = 15
data[ 4] = 14
data[ 5] = 3
data[ 6] = 2
data[ 7] = 1
data[ 8] = 10
data[ 9] = 6
data[10] = 5
data[11] = 4
data[12] = 13
data[13] = 12
data[14] = 11

---

ARRAY SORTED DUE TO CODE:

data[ 0] = 15
data[ 1] = 14
data[ 2] = 13
data[ 3] = 12
data[ 4] = 11
data[ 5] = 11
data[ 6] = 10
data[ 7] = 9
data[ 8] = 8
data[ 9] = 7
data[10] = 6
data[11] = 5
data[12] = 4
data[13] = 3
data[14] = 2

As you can see the 11 appears twice in the "sorted" code. Though even as I type this I'm wondering if the value of "high" has anything to do with it.

Answer 1

unless it is homework you can try:

std::sort(A, A + N, std::greater<int>());

Answer 2

First things first:

int sparky[14];

will give you an 14-element array sparky[0..13], not 15 elements as you seem to think.

Secondly, your inner loop isn't quite right. Yours start at index 1 and go through to index 15 whereas, because C array are zero-based, you should be cycling from 0 through to 14.

You can fix that by changing the loop condition to while (counter < howmany) and moving the counter++ to just before the end of the inner loop.

Just to clarify that point, you do actually start the loop at 0 but, because the first thing you do in the loop is counter++ before using sparky[counter], you're processing the elements starting at index 1. And, in the last run of the loop, where counter == howmany (14 as per your other comments here), you increment it to 15 and use that, which is beyond the end of the array.

And, just to clarify that clarification :-), your loop is correct if you have howmany set to 14 (i.e., one less than the number of array elements) since, as you point out in a comment elsewhere, you load up element zero before entering the inner loop. I think you do still need to set high whenever you set holder though. If that's not done, I get two 6's and two 2's in my list and no 3 or 4.

As a side issue to your comment that howmany is set to 14, I would suggest that variable names should echo their intent. You clearly have 15 elements in the array (indexes 0 through 14 inclusive). Don't take that as criticism, I'm just trying to help out.

Thirdly, (and finally, I think), you're not setting high every time you're setting holder - these should be kept in sync for your algorithm to work correctly.

Please let us know whether this is homework. If not, I'll post my solution. If so, you should work it out from the guidelines given in this, and other, answers. You should be aware that, if it is homework and you use a solution posted on a public forum on the internet (such as this one), you will almost certainly be failed for plagiarism. Do not make the mistake of thinking your educators don't check that sort of stuff.

Bottom line, while I'm happy to post my solution, you probably shouldn't use it as classwork. Top marks though for at least giving it a shot first. Most homework seeker seem to come here with nothing more than the specs :-)

Update: Since you posted the following:

Sorry to mention but this is part of an extra credit project for a class.

I guess it's classwork. So, no soup for you :-) Still, the three points above should be enough for you to fix it. If you have specific questions about them, feel free to ask in a comment attached to this answer.

Good luck.

Answer 3

yes it does...

look at where you are using holder = sparky[counter]; and sparky[high] = sparky[counter-1]

if you read your code you will see that when the value is set for the last value, counter-1 and sparky[high] are the same thing. this will happen on all values, but you only notice it on the last one

try adding some Debug code in there to say 'i am moving this value to this place' to show you what you are actually doing... it might help :)

Answer 4

Try this:

int counter=0; // inner counter 
int counter2=0; // outer counter 
int sparky[14] = {14,13,12,11,10,9,8,7,6,5,4,3,2,1}; //array set to 14 just to simplify things 
int holder; // holds the highest value 
int FoundIndex; //stores the position where it found the value holder
int temp; 
bool Found; 
while (counter2 < howmany) 
 { 
  holder= sparky[counter];
  Found = false; 
  while (counter <= howmany && !Found) 
   { 

        if (sparky[counter] >= holder) 
         { 
              holder= sparky[counter]; 
              FoundIndex= counter; 
              Found = true; 
         }
         counter++; 
   } 
   counter2++; 
   counter=counter2;
   temp = sparky[FoundIndex]; 
   sparky[FoundIndex]= sparky[counter-1]; 
   sparky[counter-1]=temp; 
 }

basically i just fixed up your inner loop and swap. but dont use it if you dont understand it. the mistakes you made were pretty trivial, so i feel comfortable with posting code for you to use... The basic idea is, if you are using this sort of sort (no pun intended) be very carefull where your indexes end up. When dealing with two counters, make sure you keep track of them and give them meaningfull variable names. counter and counter2 are not so good. Try something like outerCounter and innerCounter. Also, high has been changed to FoundIndex.

remember, trace code (code that just outputs the value of a varaible) is extremely valuable in figuring out this sort of problem... just putting in 'cout << counter << endl; showed it was going over the end of the array...

Answer 5

For SOME reason I can't edit my own code... oh well. But paxdiablo you ARE right. All I had to do was move counter++ towards the end of the nested loop. That fixed my problem. Appreciate the help.

Again, can't edit my own code to show you were right paxdiablo, but you were... darn darn