Tortoise shell
Tortoise shell

Reputation: 47

Why isn't my C# Selection Sort algorithm outputting the expected result?

So I'm trying to implement selection sort in C# but the output isn't exactly what I'm expecting, I tried tracing the loops and even brought out the good ol pen and paper but to no avail

using System;
namespace App
{
    class Application
    {
        public static void Main(string[] args)
        {
            int[] arr = {23,45,76,22,5666,845,11,2};
            foreach (int i in sort(arr))
            {
                Console.WriteLine($"{i}");
                
            }
        }
        public static int[] sort(int[] arr)
        {
            int ind = 0;
            int minInd = 0;
            int jInd = 0;
            foreach(int i in arr)
            { 
                int min = i;
                minInd = ind;
                foreach(int j in arr)
                {
                    if (i == j)
                    {

                    }
                    else
                    {
                        if (j < min)
                        {
                            min = j;
                            minInd = jInd;
                        }                        
                    }
                    jInd++;
                }
                int temp = i;
                arr[ind] = arr[minInd];
                arr[minInd] = temp;
                ind++;
                jInd = 0;
            }
            return arr;
        }
    }

}

for some info, minInd is the index of the minimum value, ind is the index of the outer loop iteraion, and jInd is the index for the inner loop itteration, i tried using the built in IndexOf from the array class but that just results in an indexoutofbounds error, could be a problem in the way C# handles array, i normally use java so maybe they handle them in different ways, also I understand that the time complexity of this algorithm is n^2 so why do we still use it.

The output is 45 76 22 5666 845 11 23 2

Upvotes: 2

Views: 130

Answers (2)

user16612111
user16612111

Reputation:

You do not need to make your method return a sorted integer array as the integer array you passed as a parameter is sorted by moving elements to different array indices after comparing consecutive elements using the selection sort algorithm. To implement a method that sorts an integer array in ascending order using the Selection Sort algorithm then the code below would work for you.

 static void SelectionSorter(int[] arr)
        {
            for(int i=0;i< arr.Length; i++)
            {
                for(int j=i+1;j<arr.Length;j++)
                {
                    if (arr[i] >= arr[j])
                    {
                        //swap the elements so smaller elements come first
                        int temp = arr[i];
                        arr[i] = arr[j];
                        arr[j] = temp;
                    }
                }
            }

Use the method to sort the elements of your array inside the Main method by calling the method above and passing your integer array as a parameter.

static void Main(string[] args){
int[] arr = {23,45,76,22,5666,845,11,2};
SelectionSorter(arr);
//print out the elements to the console
arr.ToList().ForEach(item=>Console.Write(item+" "));
//outputs 2 11 22 23 45 76 845 5666
}

Upvotes: 3

RTF
RTF

Reputation: 421

You're doing your swapping outside of the inner loop instead of within. You don't really need to iterate through every element of the array in your inner loop either (see my edits to your solution below).

Speaking of looping, don't use for-each in a case like this; use plain old for. In the case of Selection Sort it's also easier to count down the array which is expressed more clearly using a for loop. A for loop, moreover, obviates the need for many of these additional variable declarations you have.

Finally, it's generally considered bad practice to have an if with no body. If the if condition executes no code when true, check the boolean not instead and execute the code in your else block. This also eliminates the extra check of (i == j) you have which is frankly superfluous.

For learning purposes I've adapted your code to an improved implementation (though not necessarily the ideal one):

static int[] sort(int[] arr)
{
    //int ind = 0;
    //int minInd = 0;
    //int jInd = 0;
    for (int i = arr.Length - 1; i > 0; i--)
    {
        //int min = i;
        //minInd = ind;
        for (int j = 0; j < i; j++)
        {
            if (arr[j + 1] < arr[j])
            {
                int min = arr[j + 1];
                arr[j + 1] = arr[j];               
                arr[j] = min;
            }
            //else
            //{
            //    if (j < min)
            //    {
            //        min = j;
            //        minInd = jInd;
            //    }
            //}                       

            //jInd++;
        }
        //int temp = i;
        //arr[ind] = arr[minInd];
        //arr[minInd] = temp;
        //ind++;
        //jInd = 0;
    }
    return arr;
}

Upvotes: 3

Related Questions