Using custom comparator results in inconsistent ordering

Question

I am working with an Android app, and I want to print out a HashMap's keys and values in a certain manner. Let's say the following are the contents of the HashMap:

11: 000010
12: 102643
24: 877
3: 990000
h: 6008770000
m: 0800

I want to print out the HashMap keys and values in such a way that the keys with letters should be printed first alphabetically, followed by the numeric keys in ascending order:

h: 6008770000
m: 0800
3: 990000
11: 000010
12: 102643
24: 877

What I am doing right now is:

1. Get the key set and save it to an ArrayList

2. Sort the ArrayList using a comparator

3. Print out the values in the map by using the sorted list

Here's my code:

List<String> keyList = new ArrayList<>(requestMap.keySet());
Collections.sort((keyList), comparator);
for(String key : keyList) {
    Log.d(key, requestMap.get(key));
}

Comparator<String> comparator = (o1, o2) -> {
    if (o1 == null) return -1;
    else if (o2 == null) return 1;

    if(TextUtils.isDigitsOnly(o1) && TextUtils.isDigitsOnly(o2)) {
        return Integer.compare(Integer.parseInt(o1), Integer.parseInt(o2));
    }

    if(!TextUtils.isDigitsOnly(o1)) {
        return -1;
    } else {
        return o1.compareTo(o2);
    }
};

It's so far working, but for some cases I don't get the desired order. For example, for a specific Map I always get the following result:

3: 005000
4: 000000058985
12: 095508
22: 022
h: 6008770000
m: 0221
11: 000004
13: 0120
24: 877
25: 00
35: 77690088000000131D20077100000F
37: QWERTY123456
41: 00000003
42: 100000004000000
48: 456789123451       0000050201
60: 000001
61: 0201020000000045000000000300000000000015000102000000049770000000049770000000001659

I made the comparator with the idea that the alphabetical strings should be sorted first, followed by the numeric strings, but right now I'm having doubts if my logic for the comparator is correct. Can anyone point me in the right direction?

Answer 1

if(TextUtils.isDigitsOnly(o1) && TextUtils.isDigitsOnly(o2)) {
    return Integer.compare(Integer.parseInt(o1), Integer.parseInt(o2));
}

So the case where both are digits only is dealt with.

if(!TextUtils.isDigitsOnly(o1)) {
    return -1;

This is the case where the first has non-digits. This is return -1 even if the second has non-digits. When both have non-digits we should be comparing.

} else {
    return o1.compareTo(o2);

This is the case where the first is digits, so don't want to compare. Because we have excluded both are digits only, the second must be non-digits (though we shouldn't be comparing).

}

So, it looks like you certainly want to remove the !!

You should probably also cover all cases. Either have a condition whether you should cover the compare case

!TextUtils.isDigitsOnly(o1) && !TextUtils.isDigitsOnly(o2)

or nest

if (TextUtils.isDigitsOnly(o1)) {
    if (TextUtils.isDigitsOnly(o2)) {
        ...
    } else {
        ...
    }
} else {
    if (TextUtils.isDigitsOnly(o2)) {
        ...
    } else {
        ...
    }
}