Comparator implementation error

Question

My application compares file names and one user got "comparison method violates its general contract java". But I cant reproduce error.

My question: Is it ok, when my @Override compare() method return 0, which means that object are equals?

This is my code:

Two types of sort: 1) by last modified date 2) by regex digit in file name.

public class FileComparator implements Comparator<File> {
    int sortType;

    public FileComparator(int sortType) {
        this.sortType = sortType;
    }

    @Override
    public int compare(File f1, File f2) {
        int result;
        try {
            switch (sortType) {
                case Constants.SORT_BY_DATE:
                    result = f1.lastModified() > f2.lastModified() ? 1 : -1;
                    break;
                case Constants.SORT_BY_DIGID:
                    result = checkByDigit(f1, f2);
                    break;
                default:
                    result = f1.lastModified() > f2.lastModified() ? 1 : -1;
                    break;
            }
            return result;
        } catch (Exception e) {
            return 0;
        }
    }


    private static int checkByDigit(File f1, File f2) {
        String regEx;
        Pattern p;
        Matcher m1;
        Matcher m2;

        try {
            String f1Name = f1.getName().toUpperCase();
            String f2Name = f2.getName().toUpperCase();

            //  "ScanImage  _ 001"
            regEx = "(SCANIMAGE)(\\D*)(\\d+)";
            p = Pattern.compile(regEx);
            m1 = p.matcher(f1Name);
            m2 = p.matcher(f2Name);
            if (m1.find() && m2.find()) {
                return Integer.parseInt(m1.group(3)) > Integer.parseInt(m2.group(3)) ? 1 : -1;
            }

            //  "No_digits_here_001",""No_digits_here_002"
            regEx = "(\\D+)(\\d+)";
            p = Pattern.compile(regEx);
            m1 = p.matcher(f1Name);
            m2 = p.matcher(f2Name);
            if (m1.find() && m2.find()) {
                return Integer.parseInt(m1.group(2)) > Integer.parseInt(m2.group(2)) ? 1 : -1;
            }

            // We didnt find any digit, use lexicographically compare
            return f1.compareTo(f2);

        } catch (Exception e) {
            //
        }
        return 0;
    }
}

Your sorting by last modified doesn't have a zero. So if they were last modified at the same time, then your comparator would break because the order of the arguments determines their order. — matt, Apr 20 '16 at 06:47
Also note, you might use a factory method design pattern to return a specific comparator depending on the given `sortType` rather than checking for it in the `compare()` method. — Lyubomyr Shaydariv, Apr 20 '16 at 07:11

score 0 · Answer 1 · answered Apr 20 '16 at 06:43

In this specific situation you run into problems comparing names such as aaa_002, ccc_001 and bbb. Your code doesn't really decide what the correct order would be. One option is to sort all files that contain a numerical sequence after those that don't.

boolean m1find = m1.find();
boolean m2find = m2.find();

if(m1find || m2find) {
    if (m1find && m2find) {
            return Integer.compare(Integer.parseInt(m1.group(3)) > Integer.parseInt(m2.group(3)));
    } else {
            return Boolean.compare(m1find, m2find);
    }        
}

The problem could also some from lastModified times changing during sort, but it's unlikely to happen often.

Teemu, thank you. 1) Sorry, is it exists `Boolean.compare()`? I should use `file1.compareTo(file2)` in this part? 2 )You code doesnt compare zzz.jpg and rrr.jpg ? — Azamat Almukhametov, Apr 21 '16 at 04:11

score 0 · Answer 2 · answered Apr 20 '16 at 06:50

0

When you sort on last modified the order of the arguments matters, specifically when the last modified is the same time. Use Long#compare.

answered Apr 20 '16 at 06:50

matt

10,892
3
22
34

Comparator implementation error

2 Answers2