Sorting program code

Question

I have done a sorting program, it works but I am new to the coding world and I would love to hear your thoughts.

 public class CorrectPlacement { public void placer(int... numbers) { System.out.println("Before : " + Arrays.toString(numbers) + " "); for (int i = -1; i <= numbers.length - 2; i++) { int min = numbers[i + 1]; for (int number : numbers) { if (number <= min && indexofTheRight(number, numbers, i) > i) { min = number; } } numbers[indexofTheRight(min, numbers, i)] = numbers[i + 1]; numbers[i + 1] = min; } System.out.println("After : " + Arrays.toString(numbers)); } public int indexofTheRight(int element, int[] array, int range) { for (int i = array.length - 1; i > range; i--) { if (array[i] == element) { return i; } } return -1; } } 

Answer 1

A more detailed explanation on what you're doing here would have been nice.

---

 public class CorrectPlacement { public void placer(int... numbers) { 

Your names could be better overall. I'd also rather call the class and method ArraySorter.sort, for example.

---

for (int i = -1; i <= numbers.length - 2; i++) { 

I'm still an advocate for using real names for loops, like index or counter.

---

You're never checking whether you actually received an array, it could be null.

---

 for (int i = -1; i <= numbers.length - 2; i++) { int min = numbers[i + 1]; 

For easier reading and using, I'd rather go with a index - 0 .. numbers.length - 1 loop and do mutations on the index as needed inside the loop.

---

 for (int number : numbers) { if (number <= min && indexofTheRight(number, numbers, i) > i) { min = number; } } numbers[indexofTheRight(min, numbers, i)] = numbers[i + 1]; 

You seem to be doing quite a few unnecessary iterations in your logic. What you could do instead is to have a findLowerThan function which seeks the lowest index, below the current one, and returns it. From there you can get the value easily for swapping. This also allows you to do nothing if not lower value was found. Combine this with a little bit more readable loop variant and you get rather easy to follow logic:

public void sort(int... numbers) { // Simple loop which allows to easily see that the whole array is traversed // except the last item. for (int index = 0; index < numbers.length - 1; index++) { // Extract the current value. int currentValue = numbers[index]; // Find the lowest value in the remaining array. int lowestValueIndex = findLowerThan(numbers, currentValue, index); // If none was found, there's nothing to do. if (lowestValueIndex >= 0) { // Actually swap the values. int swapValue = numbers[lowestValueIndex]; numbers[lowestValueIndex] = currentValue; numbers[index] = swapValue; } } } private int findLowerThan(int[] array, int threshold, int stopAtIndex) { int lowestValue = threshold; int lowestValueIndex = -1; // Easy to read loop again, which makes it easy to see that we // traverse only a part of the array. for (int index = array.length - 1; index > stopAtIndex; index--) { int currentValue = array[index]; if (currentValue < lowestValue) { lowestValue = currentValue; lowestValueIndex = index; } } // Returning only the index allows us to signal that no element // that is lower has been found. return lowestValueIndex; } ```

Answer 2

A few things.

for (int i = 0; i < numbers.length-1; i++) 

Also, look up quick sort. Worst-case performance O(n^2) with a best-case of O(nlogn). By randomizing the pivot in the quick sort you can enhance the best case scenario.

Psuedo for recursive Quicksort:

partition(arr[], low, high) pivot = arr[high] i = low for j:= low to high-1 if arr[j] <= pivot swap arr[i] with arr[j] i = i+1 swap arr[i] with arr[high] random(arr[], low, high) r = a random number from low to high swap arr[r] and arr[high] return partition(arr, low, high) quickSort(arr[], low, high) if low < high p = random(arr, low, high) quickSort(arr, low , p-1) quickSort(arr, p+1, high)