Selection Sort Java -Arraylist

Question

I have implemented selection sort in java using arraylist. This selection sort is only for integers . Please have a look and give feedback. can we improve it ?

package selectionSort;
import java.util.ArrayList;
import java.util.Scanner;
public class SelectionSort {
private ArrayList<Integer> num_sort=new ArrayList<>();
public SelectionSort(ArrayList<Integer> userlist) {
 this.num_sort=userlist;
 sort_select();
}
public void sort_select() {
 //outer for loop to pick one element at a time
 //e.g if list is 20,12,3,4,5,6,98----this loop will first pick 20
 for(int i=0;i<this.num_sort.size();i++) {
 //assign that element as minimum
 //in our e.g 20 will be assigned min in first iteration
 int min=this.num_sort.get(i);
 int min_index=i;
 //go over the rest of the list and compare minimum by rest of list. if number smaller than min is found, then it is assigned as new min
 //so go over list starting from 12 and comparing 20 with each of them and finding the new min, in this case it will be 3
 for(int j=i+1;j<this.num_sort.size();j++) {
 if(this.num_sort.get(j)<min) {
 min=this.num_sort.get(j);
 min_index=j;
 }
 }
 if(i!=min_index) {
 exchange(i,min_index);
 }
 }
 System.out.println("Sorted entered are : ");
 for(int x:this.num_sort) {
 System.out.print(x+" ");
 }
}
// exchange the element at ith position with min
public void exchange(int current_elem,int new_min) {
 int temp=this.num_sort.get(current_elem);
 this.num_sort.set(current_elem, this.num_sort.get(new_min));
 this.num_sort.set(new_min, temp);
}
public static void main(String[] args) {
 // TODO Auto-generated method stub
 System.out.println("Enter how many numbers you want to sort");
 Scanner in=new Scanner(System.in);
 int num_count=in.nextInt();
 System.out.println("Enter the numbers you want to sort");
 ArrayList<Integer> userlist=new ArrayList<>();
 for(int i=0;i<num_count;i++) {
 userlist.add(in.nextInt());
 }
 SelectionSort s=new SelectionSort(userlist);
}

}

Answer 1

Style

1. Use proper styling. You should use more whitespaces, this will make your code more readable. Instead of num_sort=new use num_sort = new, insead of i!=min_index use i != min_index, etc
2. Follow the Java naming conventions: variable and function names should use camelCase, not snake_case.
3. You should include documentation that explains the user how the class is used. For example, without looking at the code I would not expect that creating a new instance of the class automatically sorts the list, so it is something you must make clear in the documentation.

Comments

Commenting the code is good, but too many comments can make a simple function look complicated. The general rule is that code tells you how, comments tell you why.

For example, I think int min = this.num_sort.get(i); does not need to be commented; it's pretty clear that you are getting the i-th item of the list.

Also, leave a whitespace between // and the text, it's more readable.

Code structure

It does not make sense to have this in a class; new SelectionSort(userlist) is not the way someone would expect to sort a list. I am going to make some recommendations that for the class design, but for the rest of the review I will restructurate the code so that it is more conventional.

As a user I expect select_sort to sort the list and that's all; I decide whether I want to print the list or not (what if I want to print it differently to you?). So you should move the printing code to the main function.

Class design

If you were to continue with the current class design, there must be some changes:

1. Do not sort the list in the construction. It is not the expected behaviour. Let the user call sortSelect(). Actually, I think selectionSort() would be a better name.
2. Is exchange a method that is meant to be used by the user? I don't think so, so make it private instead of public so only the class code can use it.
3. num_sort does not seem like an intuitive name; I had to look at the code to know what it is. Just calling it list would be better.
4. You don't care how the list is internally implemented, you only care that it is a list. So use the more generic List interface instead of ArrayList.
5. You are assigning your list twice: on num_sort=new ArrayList<>() and on this.num_sort=userlist;. The first one is not necessary, because it will always be replaced in the constructor. Furthermore, since you never reassign the list again, you should declare it final.

private final List<Integer> list;
public SelectionSort(List<Integer> userList) {
 this.list = userList;
}

Alternative design

1. In my opinion, your selection sort algorithm should just be one static method that takes the list as input. That would make it similar to the Collections.sort() method that already exists in Java.
2. Scanner is a resource that must be closed. You can make Java close it automatically by using a try with resources.

try (Scanner in = new Scanner(System.in)) {
 // your code
}

3. You can still keep the convenience method exchange (as static and taking the list as parameter), but since it is quite short and you only call it in one place I think you could just write the code directly. If you decide to keep the function, I would change the parameter names to just i and j, and the method name to swap (it's more common). Since it's the same doing exchange(current_elem, new_min) than exchange(new_min, current_elem) those parameter names do not make much sense. BUT you don't even need to implement that method, you can just use Collections.swap()

Final code

/**
 * Sorts the given list using a selection sort algorithm.
 * @param list The list to be sorted.
 */
public static void selectionSort(final List<Integer> list) {
 // outer for loop to pick one element at a time
 // e.g if list is 20,12,3,4,5,6,98----this loop will first pick 20
 for (int i = 0; i < list.size(); i++) {
 int min = list.get(i);
 int minIndex = i;
 // Go over the rest of the list and compare minimum by rest of list. If number smaller than min is found, then it is assigned as new min
 // So go over list starting from 12 and comparing 20 with each of them and finding the new min, in this case it will be 3
 for (int j = i + 1; j < list.size(); j++) {
 if (list.get(j) < min) {
 min = list.get(j);
 minIndex = j;
 }
 }
 if (i != minIndex) {
 Collections.swap(list, i, minIndex);
 }
 }
}
public static void main(String[] args) {
 try (Scanner in = new Scanner(System.in)) {
 System.out.println("Enter how many numbers you want to sort");
 int count = in.nextInt();
 System.out.println("Enter the numbers you want to sort");
 List<Integer> list = new ArrayList<>();
 for(int i = 0; i < count; i++) {
 list.add(in.nextInt());
 }
 selectionSort(list);
 System.out.println("Sorted entered are: ");
 for (int x : list) {
 System.out.print(x + " ");
 }
 }
}

• gss: thanks for your comments @eric.m. Will take care of all these