What is the correct way to handle IOException in a comparator?

Question

I'm using a comparator to sort files by modified date. Those files are to be processed for contained data and deleted after. I have to make sure the files are in sequential order before processing them. Below is what I have so far.

public class FileComparator implements Comparator<Path> {  
    @Override
    public int compare(Path o1, Path o2) {
        try {
            return Files.getLastModifiedTime(o1).compareTo(Files.getLastModifiedTime(o2));
        } catch (IOException exception) {
            //write to error log
            //***
        }
    }
}

*** This is where I'm stuck. I have to return an int because compare requires it but I don't want to return zero and have false equivalency when it fails.

I tried restructuring the code but then if getLastModifiedTime() fails, o1Modified and o2Modified will be null.

public class FileComparator implements Comparator<Path> {  
    @Override
    public int compare(Path o1, Path o2) {
        FileTime o1Modified = null;
        FileTime o2Modified = null;        
        try {
            o1Modified = Files.getLastModifiedTime(o1);
            o2Modified = Files.getLastModifiedTime(o2);
        } catch (IOException exception) {
            //write to error log

        }

        return o1Modified.compareTo(o2Modified);

    }
}

Is there any standard way to handle situations like this?

If you receive an `IOException`, the implication is that something is wrong with your path(s). If one (or both) of your paths point to a file that doesn't exist, would it not make sense to say they're unequal? You can sort that how you wish (by returning either -1 or 1, depending on the exact case). Another way to put it is that all invalid Paths are equally invalid. — Hypino, Dec 13 '16 at 18:26

score 4 · Accepted Answer · edited May 23 '17 at 10:30

4

I think the way to tackle this particular situation is by calling Files.getLastModifiedTime() on each path exactly once, storing the results and then using the stored results during the sorting.

This has several benefits:

It cleanly solves the IOException problem.
It doesn't repeatedly and unnecessarily perform the same costly I/O operations on the same files.
It ensures consistent ordering even if the last modification time of a file changes midway through the sort (see, for example, "Comparison method violates its general contract!" for how this could subtly break your code).

edited May 23 '17 at 10:30

Community

1
1

answered Dec 13 '16 at 18:28

NPE

486,780
108
951
1,012

1

Good answer that solves the problem at the root cause instead of a band-aid. – HNA Dec 13 '16 at 18:43

score 2 · Answer 2 · answered Dec 13 '16 at 18:28

Throw a runtime exception wrapping the IOException:

    try {
        return Files.getLastModifiedTime(o1).compareTo(Files.getLastModifiedTime(o2));
    } catch (IOException e) {
        throw new UncheckedIOException("impossible to get last modified dates, so can't compare", e)
    }

Note however that the modified time could change during the sort, which would make your comparator incorrect: it wouldn't respect its contract anymore. So a better approach would be to iterate through your paths first, and wrap them into some TimedPath object which would store the last modification time, and then sort those TimedPath objects.

Thank you for suggesting a TimedPath object. NPE's answer solves my problem and your suggestion of creating a time aware object improves the solution. — HNA, Dec 13 '16 at 18:40

What is the correct way to handle IOException in a comparator?

2 Answers2