Is this a proper selection sort?
Any code issues?
public static void SelectionSort(ref int[] a)
{
int n = a.Length;
for (int j = 0; j < n - 1; j++)
{
int iMin = j;
for (int i = j + 1; i < n; i++)
{
if (a[i] < a[iMin])
{
iMin = i;
}
}
if (iMin != j)
{
int temp = a[j];
a[j] = a[iMin];
a[iMin] = temp;
}
}
}
//test
int[] ar = new int[] { 90, 90, 71, 82, 93, 75, 81, 54, 36, 102, 99, 34, -56, 103, 78, 796, 52, -56, 5, 215 };
SelectionSort(ref ar);
foreach (int i in ar)
Debug.WriteLine(i);
2 Answers 2
- The parameter does not need to be a
ref
, since there is no assignment to the variablea
in theSelectionSort
method. - There are too few tests.
- The current test data looks entirely random, as if you had no idea what the interesting cases were.
- The tests should at least cover some interesting edge cases, like:
- An empty array
- An array with only 1 element
- An array with 2 elements
- An array with 10 equal elements
- An array that is sorted ascendingly
- An array that is sorted descendingly
- An array that is random
- An array containing
int.MinValue
, 0,int.MaxValue
- An array of 5 elements, in which every permutation is tested whether it sorts to the same results
- The test code should be presented in the question as real test methods, not just a code snippet.
-
\$\begingroup\$ Correct it does not need ref. Someone commented I did not test so I added that. I did test other cases. \$\endgroup\$paparazzo– paparazzo2017年07月29日 15:37:38 +00:00Commented Jul 29, 2017 at 15:37
As it's already been said, using ref
as a parameter does not have any advantages here but... it can be used as the new C# 7 feature with return values and local variables which I find allows us to greatly improve the readability of the algorithm. By using it this way we can now return a reference to an array item and also use ref
local variables that we can overwrite. We no longer have to work with indexes everywhere. Setting the variables will set array items.
public static void SelectionSort(int[] collection)
{
var n = collection.Length;
for (var i = 0; i < n - 1; i++)
{
ref var currentValue = ref collection[i];
ref var minValue = ref FindMinValue(collection, i);
if (minValue != currentValue)
{
var temp = currentValue;
currentValue = minValue;
minValue = temp;
}
}
}
private static ref int FindMinValue(int[] collection, int offset)
{
var iMin = offset;
for (var i = offset + 1; i < collection.Length; i++)
{
if (collection[i] < collection[iMin])
{
iMin = i;
}
}
return ref collection[iMin];
}
ref
keyword for thea
parameter? Since you just set values of an array it is useless. As for downvote I think it is because of the task itself. Selection sort is one of the first algorithms when people start to learn programming. You doesn't look like a beginner so it is really strange to see this post from you :) Of course there are tons of code examples of this sort on all programming languages including C#. \$\endgroup\$