Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 0 down vote favorite

Hello I am working on a program in which 10000 random no repeat numbers are selectively sorted into ascending order.

I have written the following code.

import java.util.Random;


public class Sorting {
public static void main (String[] args){
//Here, we initiate an array with the integers
//1 through 10,000 in order.
    Random rgen = new Random();
    int[] intArray = new int[10000];
    for (int i=0; i<10000; i++) {
        intArray[i] = i+1;
    }
//Here, we randomize the positions.
    for (int i=0; i<10000; i++) {
        int randomPosition = rgen.nextInt(10000);
        int temp = intArray[i];
        intArray[i] = intArray[randomPosition];
        intArray[randomPosition] = temp;
}
//Here, we check for the maximum in the 
//unsorted section of the list.
    int max;
    for (int i=9999; i>=0; i--) {
        max = i;
        for (int j=i-1; j>=0; j--)
            if (intArray[j]>intArray[max]) {
                max = j;
            }
//Here, we swap the positions of the 
//maximum and the last value in the 
//unsorted section of the list.
        if (max != i) {
            int tmp = intArray[i];
            intArray[i] = intArray[max];
            intArray[max] = tmp;
        }
    }
//Here, we output the result.
    for (int k=0; k<10000; k++) {
        System.out.println(intArray[k]);
    }
}
}

I know I asked a similar question earlier but I would appreciate any help.

share|improve this question
Ok I will ask on codereview – Fernando Martinez Apr 28 at 2:00
what actually is your question? – suspectus Apr 28 at 10:34
Sorry I am wondering how to improve it. – Fernando Martinez Apr 28 at 16:38

2 Answers

up vote 2 down vote

Your code for finding the maximum of an array is wrong. It should be

int maxIndex = 0;
for(int k = 1; k < intArray.length; k++)
    if(intArray[k] > intArray[maxIndex])
        maxIndex = k;

Additionally, you will have to run the sorting code intArray.length times if you want the array to be truly sorted. This type of sort takes O(n^2) time, as opposed to O(nlogn) of quicksort.

share|improve this answer
I tried doing that code but I mess up on the swapping. – Fernando Martinez Apr 28 at 17:11
@FernandoMartinez How so? What is happening with the swapping? – Jared Nielsen Apr 28 at 20:19
up vote 0 down vote

I tested it and it works as it should. I would only like to remark a couple of things.

You may want to split your long commented function in short ones ie:

  • static int[] InitArray(int length){...}

  • static int[] SelectionSort(int[]arr){...}

  • static void Swap(int[]arr, int i, int j){...}

  • print the result in the main, as you do know.

This makes your code easier to read and easier to reuse.

Another thing I'd remove is the if(max != i). I'd rather make the swap everytime, if you look carefully there's small chance that you do this operation unnecesarily and when you do it, it won't harm you.

Note: If you keep doing the swap as you do now, you won't have any problem by removing the if. However if you modify it to make the swap without a temp variable it might go wrong.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.