2
\$\begingroup\$

I need to sort files by the type of the File. To accomplish the task I first wrote the code below:

private static Comparator SortByFileType() {

    return Comparator.comparing((File f) -> {
        String probeContentType = null;
        try {
            probeContentType = Files.probeContentType(f.toPath());
            if (probeContentType ==null) {
                probeContentType = "probeContentTypeIsNull";
            }
        } catch (IOException e) {
        }
        return probeContentType;
    });
}

Then I needed to perform different sorts, so I decided to make an enum of sorts, but I wasn't able to pass the above code to the enum. I instead rewrote the above code as given below:

public enum Sort implements Comparator<File> {

/**
 * Compare files by the name of the File A<->Z
 *
 */
ByFileName {



        },
/**
 * Compare Files by the Type of file
 */
ByFileType {

        @Override
        public int compare(File f1, File f2) {
            String fileType1 = "";
            String fileType2 = "";

            try {
                fileType1 = Files.probeContentType(f1.toPath());
                fileType2 = Files.probeContentType(f2.toPath());
            } catch (IOException ex) {

            }

            if (fileType1 == null) {
                fileType1 = "probeContentTypeIsNull";
            }
            if (fileType2 == null) {
                fileType2 = "probeContentTypeIsNull";
            }

            return fileType1.compareTo(fileType2);
        }

    }
}

Now I need to know two things:

  1. Can we turn the code passed to the ByFileType enum into a lambda?
  2. Can the code passed to the ByFileType be optimized and improved?
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

This is a great concept to work with. Creating an enum of comparators is a clever way to make it easy to manage the sort order of file collections.

The name concerns me. I realize that Sort is convenient, but enums are normally given a "noun" name. "Sort" is a verb - something you do, not a noun which is a "thing". I would call it Sorter, or even FileSorter. Adding File to it makes code more readable.

Additionally, enum members are normally in all-uppercase and under-score separated.... so, BY_FILE_NAME instead of ByFileName.

Adding the "File" to be FileSorter you no longer need to have File as part of the enum names, so you will have: FileSorter.BY_NAME.

I believe you have truncated some code from the ByFileName enum. It does not compile without the override of the compare method. I'll just claim it is because of how you summarized things for this question.

Now, about the functional aspects. I agree, the override is a mess, and can be sorted out quite easily. First up, what you want to do is pass a comparator in as the constructor for the enum, and then use that comparator as a tool in the compare function.... like this:

private final Comparator<File> comp;

private FileSorter(Comparator<File> comp) {
    this.comp = comp;
}

@Override
public int compare(File o1, File o2) {
    return comp.compare(o1, o2);
}

That makes the actual enum guts really simple, not it's just a case of setting up the enum members.... the ones that are easy are things like size, and name sorts:

BY_NAME(Comparator.comparing(File::getName))

and

BY_SIZE(Comparator.comparingLong(File::length))

Taking your base code though, for sorting by type, you had:

return Comparator.comparing((File f) -> {
    String probeContentType = null;
    try {
        probeContentType = Files.probeContentType(f.toPath());
        if (probeContentType ==null) {
            probeContentType = "probeContentTypeIsNull";
        }
    } catch (IOException e) {
    }
    return probeContentType;
});

That code is 'ugly' because it is hard to follow the logic through, the empty catch block is a mess, and the blocked lambda (File f) -> {....} type expressions make me want to create a function for them, so I do....

I created the function:

private static final String extractType(File f) {
    try {
        String probeContentType = Files.probeContentType(f.toPath());
        return probeContentType == null ? "probeContentTypeIsNull" : probeContentType;
    } catch (IOException e) {
        return null;
    }
}

Note how I have a correctly-scoped probeContentType variable, and I use a return in both the successful, and catch-side of the try/catch block. The logic is now very visible. That function extracts just the file's type. How is it used, though?

BY_TYPE(Comparator.comparing(FileSorter::extractType)),

Putting this all together, I have the following code which I believe is quite simple, and readable:

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.util.Comparator;

public enum FileSorter implements Comparator<File> {

    BY_TYPE(Comparator.comparing(FileSorter::extractType)),
    BY_NAME(Comparator.comparing(File::getName)),
    BY_SIZE(Comparator.comparingLong(File::length));

    private static final String extractType(File f) {
        try {
            String probeContentType = Files.probeContentType(f.toPath());
            return probeContentType == null ? "probeContentTypeIsNull" : probeContentType;
        } catch (IOException e) {
            return null;
        }
    }

    private final Comparator<File> comp;

    private FileSorter(Comparator<File> comp) {
        this.comp = comp;
    }

    @Override
    public int compare(File o1, File o2) {
        return comp.compare(o1, o2);
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ A very explanatory and detailed answered. +1000 I wish I could \$\endgroup\$
    – SSC
    Jul 23, 2015 at 17:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.