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.