Skip to content
Advertisement

Why isn’t my selection sort program working?

I am trying to create my own program to do selection sort on an array of integers. I have come up with the following program, which works on some arrays, but not on others, such as this one. I have been trying to trace the problem, and I think it might have to do with where I am placing the min = num [x]; line. However, I am not sure where I should move it in order to fix the problem. Does anyone have any suggestions? Thank you.

p.s. I have provided some of my test cases and their results at the bottom.

        int [] num = {4, 9, 7, 6, 0, 1, 3, 5, 8, 2};

        int min = num [0];
        int temp;
        int index = 0;

        for (int x = 0; x < num.length; x ++)
        {
            min = num [x];
            temp = num [x];
            for (int y = x + 1; y < num.length; y ++)
            {
                if (num [y] < min)
                {
                    min = num [y];
                    index = y;
                }
            }
            num [x] = min;
            num [index] = temp;
            min = num [x];
        }
Output Test Cases:
array: {4, 9, 7, 6, 0, 1, 3, 5, 8, 2}
result: [0, 1, 2, 3, 4, 4, 5, 7, 8, 8]

array: {7, 5, 8, 9, 1, 6, 3, 0, 2, 4}
result: [0, 1, 2, 3, 4, 5, 6, 7, 7, 8]

array: {9, 8, 7, 6, 5, 4, 3, 2, 1, 0}
result: [0, 1, 2, 3, 4, 9, 6, 7, 8, 9]

Advertisement

Answer

there is one big problem and multiple small ones, see comments:

int [] num = {4, 9, 7, 6, 0, 1, 3, 5, 8, 2};

int min = num [0]; // you don't need this variable and assignment here, you can declare it inside loop
int temp; // the same for this variable
int index = 0; // the same here

for (int x = 0; x < num.length; x ++)
{
    min = num [x];
    temp = num [x];
    // here you need to re-initialize index variable to some value, otherwise it could be some garbage from previous iterations
    for (int y = x + 1; y < num.length; y ++)
    {
        if (num [y] < min)
        {
            min = num [y];
            index = y;
        }
    }
    num [x] = min;
    num [index] = temp;
    min = num [x]; // this assignment does not make sense as you're overwriting it next iteration
}

you can simplify code a little bit:

for (int x = 0; x < num.length; x++) {
    int index = x;
    for (int y = x + 1; y < num.length; y++)
        if (num[y] < num[index])
            index = y;
    int temp = num[x];
    num[x] = num[index];
    num[index] = temp;
}
User contributions licensed under: CC BY-SA
8 People found this is helpful
Advertisement