0

I'm wondering if anyone could show me where my logic in this selection sort (yes it's for educational use, I realize that there's an Arrays.sort() method) is flawed.

 public static void selectionSortByName() throws IOException {
    String temp;
    for (int i = 0; i <= nameArraySize; i++){

        String smallest = name[i];


        for (int j = 0; j <= nameArraySize; j++){
            if (name[j].compareTo(name[i]) < 0){
                temp = smallest;
                name[j] = temp;
                name[i] = smallest;
            }
        }

    }
  }

I'm getting a NullPointerException on my line with the compareTo method, so I would expect that I just have a logic error in one of my conditionals.

Any help would be great, thanks!

SillyPointer
  • 27
  • 2
  • 5
  • Does your `name` array contain any `null`-elements? Is `name` itself `null`? That sort of thing could create a `NullPointerException`. – Ghostkeeper Feb 27 '14 at 01:20
  • Change `<=` to `<`, and i think that your selection sort is not a selection sort – nachokk Feb 27 '14 at 01:22
  • @nachokk That's for Find My Error Round #2 ;-) – user2864740 Feb 27 '14 at 01:25
  • Thank you for the suggestion :). Strings in my name[] are read in from a text file with a scanner, in that method I made sure to exclude null's; also on an output of the array I only see Strings. So I don't believe so. – SillyPointer Feb 27 '14 at 01:26
  • @user3358417 If you "read into" an array then it's *most definitely* full of nulls at the end for the elements which are *not* set - this is because arrays have a *fixed size*, so unless you've done special magic to "regrow" or "shrink" or "created it the perfect size to begin with" this is just to be expected. I recommend reading into an ArrayList and then *converting* it to an array if required. – user2864740 Feb 27 '14 at 01:26
  • Ahhh, I wasn't aware. Thank you for the information and suggestion :) I guess that pretty much solves my problem. Thank you all. – SillyPointer Feb 27 '14 at 01:32

2 Answers2

2

Obviously, your nameArraySize does not fit the real length of array name or some cells in your name[i] is equal to null.

Altough, I dont see a point in using some redundant variables, if you can use this :

for (int i = 0; i < name.length; i++)

EDIT :

Ah and now I see it, you have <= instead of <

libik
  • 22,239
  • 9
  • 44
  • 87
  • I tried it and it does : "iejge".compareTo(null); – libik Feb 27 '14 at 01:27
  • You're correct - for some reason I thought it resulted in a value when the argument supplied was null. (And the darn javadoc is clearly missing a documentable exception :-/) – user2864740 Feb 27 '14 at 01:29
0

1) throws IOException is not necesary at all!. Your code doesn't throw this checked exception.

2) Change nameArraySize to name.length

3) As i said in comments change <= to <

4) AFAIK selectionSort doesn't swap until it finds the smallest one.

So your code good look like:

public static void selectionSortByName()  {

    for (int i = 0; i < name.length; i++){

        int smallest =i;
        //invariant is sorted in all n < [i]
        for (int j = i+1; j <= nameArraySize; j++){
            if (name[j].compareTo(name[smallest]) < 0){
                smallest=name[j];
            }
        }

       if(!name[smallest].equals(name[i])){
          swap(name,i,smallest);
       }

    }
  }

  public static void swap(String [] name,int i, int j){                
                String temp = name[j];
                name[j] = name[i];
                name[i] = temp;
  }
nachokk
  • 14,363
  • 4
  • 24
  • 53