0

I'm new to Java, and I'm trying to understand Comparator Interface. I tried the following code:

package comparator;

import java.io.*;
import java.util.*;

public class comparator {
    public static void main(String args[]){
        bankAccount[] ba=new bankAccount[500];
        ba[0]=new bankAccount(50);
        ba[1]=new bankAccount(90);
        ba[2]=new bankAccount(20);
        Comparator c=new comparing();
        System.out.println(c.compare(ba[0], ba[1]));
        Arrays.sort(ba, c);
    }
}

class bankAccount{
    public bankAccount(double bal){
        balance=bal;
    }
    public double balance;
}

class comparing implements Comparator{
    public int compare(Object first, Object second){
        bankAccount ba1=(bankAccount)first;
        bankAccount ba2=(bankAccount)second;
        int retval;
        if(ba1.balance<ba2.balance){
            retval=-1;
        }
        else if(ba1.balance>ba2.balance){
                retval=1;
            }
        else{
            retval=0;
        }
        return retval;
    }
}

I'm getting the following error message:

Exception in thread "main" java.lang.NullPointerException
    at comparator.comparing.compare(comparator.java:29)
    at java.util.TimSort.binarySort(TimSort.java:265)
    at java.util.TimSort.sort(TimSort.java:208)
    at java.util.TimSort.sort(TimSort.java:173)
    at java.util.Arrays.sort(Arrays.java:659)
    at comparator.comparator.main(comparator.java:13)

Can I know what the mistake is. How should I use the Arrays.sort() method. Any help would be sincerely appreciated.

Mureinik
  • 297,002
  • 52
  • 306
  • 350
user1658435
  • 574
  • 1
  • 9
  • 28
  • You should do some sort of `null` check in your `comparing` class. Also you should do a check on the `first` and `second` objects in your `comparing` class to make sure they're `bankAccount` objects. You don't want to just automatically cast them to be `bankAccount` objects. – Franklin Nov 06 '13 at 05:21
  • Refer to these questions for good example and explanation : http://stackoverflow.com/questions/2839137/how-to-use-comparator-in-java-to-sort http://stackoverflow.com/questions/2266827/when-to-use-comparable-and-comparator – Barun Nov 06 '13 at 05:31

5 Answers5

8

In your main method, you've declared a 500-length array but have only initialized 3 of the elements, so 497 of them are null. This is causing problems in your comparator because the comparator is called for many pairs of elements, including some of the null ones.

Try modifying your main method to only declare and initialize a 3-length array and compile and run it again. Maybe you do have a problem in your comparator, but at least let's remove the obvious issue and try doing the sort with all elements being non-null.

Platinum Azure
  • 45,269
  • 12
  • 110
  • 134
  • Good job implementing your `Comparator` right the first time, then. :-) – Platinum Azure Nov 06 '13 at 05:30
  • 1
    I'm glad you're enjoying the learning experience! :-) You should look at some of the other answers for other best practices to follow in the future (for example, doing a `null` check or checking that the instances are of the correct class). – Platinum Azure Nov 06 '13 at 05:36
1

In addition to the previous answer by Platnium Azure, you should also use generics in your Comparator subclass like this:

class comparing implements Comparator<bankAccount>
{
...
    @Override
    public int compare(bankAccount first, backAccount second)
    {
    ...
    }

    @Override
    public boolean equals(Object obj)
    {
    ...
    }
...
}

This will enforce the use of your comparing class to be used to sort only bankAccount instances.

SimonT
  • 2,219
  • 1
  • 18
  • 32
  • There are multiple things wrong with this answer. First you would want to `implement` the `Comparator` as opposed to `extends` it since it's an interface. Also, you aren't using generics at all in your example. Given your code, you're basically saying that the `Comparator` is of type `bankAccount`. If it were truly generic, it would look something like `Comparator` along with some other changes, but that's beside the point. – Franklin Nov 06 '13 at 05:33
  • @Franklin: Care to elaborate on what those are? I spot an issue around how type erasure works and some slight laziness on the curly brace front, but what am I missing? – Platinum Azure Nov 06 '13 at 05:38
  • The part about the `implements` was correct, I made a mistake there. I don't really know what you mean by the part about the generics though, could you explain where the problem is? – SimonT Nov 06 '13 at 22:19
  • @SimonT to make this fully generic, you'd have to do something like `class Comparing implements Comparator{...}`, which reads `Comparator` of type `T`, which is generic. `class Comparing implements Comparator{...}` is a `Comparator` of type `bankAccount`, which isn't generic since it's only applicable to `bankAccount` objects. – Franklin Nov 08 '13 at 21:50
  • 1
    Franklin, I think it's just a matter of miscommunication here. My intention was to make `comparing` only accept `bankAccount` instances, since the code he provided only works if the given `Object`s turn out to be `bankAccount`s. I meant to say that some use of Java's generics (as in the diamond notation) would be helpful. Thanks for the pointers, though. – SimonT Nov 08 '13 at 23:18
0

The problem is in the fact that you have only filled the first 3 entries of the 500 entry array, the sort method runs through those and compares just fine until it meets entry 4, which is null, throwing NPE when it tries to call its compareTo(..) method.

You can fix it by either filling the array or just giving it a length of 3.

arynaq
  • 6,710
  • 9
  • 44
  • 74
0

Change the size of the bankAccount array to 3.

if any array element is null, it will throw null pointer exception if your comparator doesnt support null;

The below code will work with array size 500 because comparator handle null case.

    if (first == null || second == null)
    {
        return 0;
    }

Modified Original Class Code Starts with the fix(lookout for the changes in the comparing class):

import java.io.*;
import java.util.*;

public class comparator {
    public static void main(String args[]){
        bankAccount[] ba=new bankAccount[500];
        ba[0]=new bankAccount(50);
        ba[1]=new bankAccount(90);
        ba[2]=new bankAccount(20);
        Comparator c=new comparing();
        System.out.println(c.compare(ba[0], ba[1]));
        Arrays.sort(ba, c);
    }
}

class bankAccount{
    public bankAccount(double bal){
        balance=bal;
    }
    public double balance;
}

class comparing implements Comparator{
    public int compare(Object first, Object second){
        bankAccount ba1=(bankAccount)first;
        bankAccount ba2=(bankAccount)second;
        int retval;
        if (first == null || second == null)
        {
            return 0;
        }
        if(ba1.balance<ba2.balance){
            retval=-1;
        }
        else if(ba1.balance>ba2.balance){
                retval=1;
            }
        else{
            retval=0;
        }
        return retval;
    }
}
Raghu
  • 1,363
  • 1
  • 23
  • 37
0

Your array is not fully initialized, if you don't wanna initialize it then change your comparing

class comparing implements Comparator{
public int compare(Object first, Object second){
    bankAccount ba1=(bankAccount)first;
    bankAccount ba2=(bankAccount)second;
    int retval;
    if (ba1==null || ba2 == null) {
        retval=0; // return what you want if bankAccount is null.
    }
    else if(ba1.balance<ba2.balance){
        retval=-1;
    }
    else if(ba1.balance>ba2.balance){
            retval=1;
        }
    else{
        retval=0;
    }
    return retval;
}
}
Sumit Singh
  • 15,743
  • 6
  • 59
  • 89
  • I personally think it's a good idea not to return equal in the case of a `null` item... Instead, depending on if you want `null` values first or last, you should return a value such that the `null` item is always considered "less than" or "greater than" the non-`null` item. (I usually favor making the `null` "greater" so it goes to the end and the non-`null` values are first in an ascending sort. Of course, better still would be to pre-check that the entire array is initialized first before even sorting!) – Platinum Azure Nov 06 '13 at 05:48