Selection Sort Java and Analysis

Question

I have written a selection sort code in java. I know its very elementary algorithm, but since i am learning, so wanted your input about the quality of the code. Please have a look at the code: package selection_sort;

import java.util.Scanner;

public class SelectionSort {

int [] arrayToBeSorted;
Scanner scan=new Scanner(System.in);


SelectionSort(){
    System.out.println("Enter the number of elements");
    int total=scan.nextInt();
    this.arrayToBeSorted=new int[total];
    for(int i=0;i<total;i++) {
        System.out.println("Enter the "+i+" number of elements");
        this.arrayToBeSorted[i]=scan.nextInt();
    }
    
}

public void Sort() {
    int minIndex,min;
    boolean swapRequired=false;
    for(int i=0;i<arrayToBeSorted.length;i++) {
        minIndex=i;
        min=this.arrayToBeSorted[i];
        for(int j=i+1;j<this.arrayToBeSorted.length;j++) {
            if(this.arrayToBeSorted[j]<min) {
                min=this.arrayToBeSorted[j];
                minIndex=j;
                swapRequired=true;
            }
        }
        if(swapRequired) {
            swap(i,minIndex);
        }
                    
    }
    
    for(int x:this.arrayToBeSorted) {
        System.out.print(x+" ");
    }
        
    }

public void swap(int i,int j) {
    
    //System.out.println("swap called, pos="+i+"and minindex="+j);
    int temp=this.arrayToBeSorted[i];
    this.arrayToBeSorted[i]=this.arrayToBeSorted[j];
    this.arrayToBeSorted[j]=temp;
    
    //System.out.println("------------");
    
}
public static void main(String[] args) {
    SelectionSort s=new SelectionSort();
    s.Sort();
}

}

Also about Analysis, as selection is O(n2), is it because I am using nested for loop. Is there any tool which tells us about complexity of code .Thanks in advance and please be patient if its very naive

Answer 1

int [] arrayToBeSorted;
Scanner scan=new Scanner(System.in);

Without any modifier, members and functions are package-private. Which means that they are accessible from the same package, but not by instances extending the class. That's an extremely odd thing, actually, when looking at it from an object-oriented point of view. You want to either make them private, or if extending classes should be able to access them, protected.

Same goes for the constructor.

---

    System.out.println("Enter the number of elements");
    int total=scan.nextInt();
    this.arrayToBeSorted=new int[total];

You could use a List/ArrayList instead of an array, which would mean that you can receive as many numbers from the users as they want without having to specify the count beforehand. An empty input could be used for declaring the end of the list. However, that has the downside that you must use Integer instead of int, so it' is kinda hard to tell what is better. It also has the downside that detecting an empty input is that readily available from Scanner, as you'd need to use nextLine for that, with manual conversion to int.

You could also emulate a List by dynamically growing the array. Either by doing it for every item, performance does not matter in this use-case, or by doubling the size when needed.

---

this.arrayToBeSorted=new int[total];

You only require the this modifier if you have a variable with the same name in the same scope, for example in a constructor:

public ValueContainer(int value) {
    this.value = value;
}

It's common practice to omit this if not needed. If you ever find yourself in the situation that it is confusing whether an instance, static or local variable is used, you did something wrong there anyway.

---

for(int i=0;i<total;i++) {

I'm a very persistent advocate for using "real" names for variables in loops.

for (int counter = 0; counter < total; counter++) {
// or
for (int index = 0; index < total; index++) {

---

public void Sort() {

The Java naming conventions state that methods/functions should be lowerCamelCase.

---

int minIndex,min;

Personally I'd avoid declaring multiple variables on the same line. It makes it easy to miss declaration.

---

boolean swapRequired=false;

That's a great example of a great name for a variable, thank you!

---

    for(int x:this.arrayToBeSorted) {
        System.out.print(x+" ");
    }

That, unfortunately, is a bad name for a variable. Only use "x", "y" and "z" when working with dimensions. "value" or "number" would be a great name here.

---

Overall, it looks valid. I did not run it to test the functionality, though. What you should change is that you have logic in the constructor. Nobody expects constructors to be filled with logic, because you can't see the intent of logic from them. They should be as nothing doing as possible. That means that you should move the logic into sort or, even better, into main. Your Sorter should not be concerned with input at all, it should only be concerned with sorting values, ideally, and another class (InputReader?) should be concerned with getting the input. So what you could do is something like this:

public static final void main(String[] args) {
    InputReader inputReader = new InputReader();
    
    int[] values = inputReader.readValues();
    
    Sorter sorter = new Sorter();
    sorter.sort(value);
    
    // TODO Print sorted output.
}

That also has the upside that you don't need to hold state in Sorter at all, which would make it thread-safe by default.