what the wrong in this code

Question

0.00/5 (No votes)

See more:

why this code doesn't do the 2nd iteration?

C++

#include <iostream>
using std::cout;
using std::endl;

void selectionSort( int * const, const int ); 
void swap( int * const, int * const ); 
int main(int argc, char *argv[])
{
    const int arraySize = 8;
    int a[ arraySize ] = { 2, 5, 12, 1, 9, 6, 3, 10 };

    selectionSort( a, arraySize ); 

     
    for ( int j = 1; j < arraySize; j++ )
        cout << a[ j ] << " ";

    cout << endl;
    system("PAUSE");
    return EXIT_SUCCESS;
}
 
  
void selectionSort( int * const array, const int size )
{
    
       
    int smallest; 
    int max;
    int m=0;
    int i=1;
     
    while( i < size - i && m < 2)
    {
       
        smallest = i; 
        max=i;
        
        for ( int index = i + 1; index < size-i+1; index++ )
        {
            if ( array[ index ] < array[ smallest ] )
                smallest = index;
        }
              
        for ( int x = i + 1; x < size-i+1; x++ )
        {
            if ( array[ x ] > array[ max ] )
                max = x;
        }
              
        swap( &array[ i ], &array[ smallest ] );
        swap( &array[ size-i ], &array[ max ] );
        i++;
        m++;
    }
    
      
}                       
void swap( int * const element1Ptr, int * const element2Ptr )
{                                                            
    int hold = *element1Ptr;                                  
    *element1Ptr = *element2Ptr;                              
    *element2Ptr = hold;                                      
}

Posted 16-May-12 8:09am

Member 8840372

Updated 16-May-12 8:45am

lewax00

v3

Add a Solution

Comments

Richard MacCutchan 16-May-12 14:21pm

My guess: there's a bug in there somewhere.
My advice: use your debugger to find out.

OriginalGriff 16-May-12 14:51pm

:thumbsup: LOL

3 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

OriginalGriff · Answer 1 · 2012-05-16T08:50:00

The first thing you need to do is sort out the indentation - it is all over the place and that makes your code very hard to read. If you press CTRL+A to select it all, then CTRL+K CTRL+F Visual Studio will sort it out for you.

Then, start by looking at your loops - or hadn't you noticed it only printed 7 out of the 8 elements when it finished?

When you have sorted that out, change your variable names. What are "i" and "m" supposed to do? "i" is acceptable (just) as an unimportant straight index, but you have "index" for that, and "i" doesn't do that job anyway.

Looking at your code, and it's copious comments, I have no idea what "m" is there for - I know it is probably something to do with your problem, since it restricts the outer loop to two passes - which means you can only swap eight values, so if you pass your method ten elements it'll fail big time.

You need to go back to your algorithm, I think, and start looking at that as well.

maj000 · Answer 2 · 2012-05-17T03:03:00

Just to add to lewax's suggested changes.

The first swap (of the smallest and ith array elements) should be moved to after the first for loop (where the index of the smallest array element is found). Otherwise, selectionSort fails when the ith element is the largest, and is swapped for the smallest before it can be swapped for the largest.

C++

void selectionSort( int * const array, const int size )
{
    int smallest;
    int max;
    int i=0;
    
    while( i < size - i )
    {
        smallest = i;
        max = i;
        
        for ( int index = i; index < size-i; index++ )
        {
            if ( array[ index ] < array[ smallest ] )
                smallest = index;
        }

        swap( &array[ i ], &array[ smallest ] );
        
        for ( int x = i; x < size-i; x++ )
        {
            if ( array[ x ] > array[ max ] )
                max = x;
        }

        swap( &array[ size - ( i + 1 ) ], &array[ max ] );

        i++;
    }
}

lewax00 · Answer 3 · 2012-05-16T08:42:00

You have a few issues here.

First, arrays start at 0, not one, so both your loop in main and in selectionSort should start at 0, not at 1.

Next, I'm not sure why your loops in selection sort go to "size-i+1" and in the case of i=0 it goes out of bounds, "size-i" works fine.

On the other hand, size is one greater than the maximum index of the array (in this case, the indexes go from 0-7) so the second swap should use the index of "size-i-1" instead of just "size-i".

I think those were the only changes I made to get it working.

EDIT: OriginalGriff's solution reminded me, I took out the m variable in selectionSort as well. Wasn't sure what it did and it worked fine without it.

what the wrong in this code

3 solutions

Solution 2

Solution 3

Solution 1

Add your solution here

Preview 0