Use lines() stream Lambda but need to close

Question

What I want to do in my application is that let user chooses a series of text files by using filechooser, and then my application will filter some of them if it does't contain specified subString.

It's my code by using lambda:

    FileChooser fileChooser = new FileChooser();
    fileChooser.setTitle("choose files");
    fileChooser.getExtensionFilters().add(new FileChooser.ExtensionFilter("TXT files (*.txt)", "*.txt"));
    fileChooser.getExtensionFilters().add(new FileChooser.ExtensionFilter("CSV files (*.csv)", "*.csv"));
    List<File> targetList = fileChooser.showOpenMultipleDialog(primaryStage);
    if (targetList == null) {
        System.out.println("targetList none");
    }
    targetList.parallelStream()
            .filter(f->{
                        try {
                            return new BufferedReader(new FileReader(f)).lines()
                                    .anyMatch(line-> (line.indexOf(",")!=-1));
                        } catch (FileNotFoundException e) {
                            e.printStackTrace();
                        }
                        return false;
                    }
            )
            .collect(Collectors.toList());

It wroks, but not perfect! Because BufferedReader is never closed and I need to wrap it with a Buiky catch statesment.

Anyone could give tips to improve it in elegant way?

You can use [tr with resource](https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html) — thopaw, Mar 08 '17 at 08:02

Lii · Accepted Answer · 2018-03-26T10:01:44.070

This can be solved in an elegant way using the same technique as in this answer.

It is possible to create a utility function that applies a function and then closes a resource reliably using a try-with-resource-statement.

Using that your code would look like this:

List<File> filteredList = targetList.parallelStream()
    .filter(f -> f.exists())
    .filter(f -> applyAndClose(
        () -> new BufferedReader(new FileReader(f)),
        reader -> reader.lines().anyMatch(line-> (line.indexOf(",") !=-1))))
    .collect(Collectors.toList());

Note that you have to save the result of the collect method, otherwise that is lost! Also, I test for file existence in a separate step.

The utility function itself looks like this:

/**
 * Applies a function to a resource and closes it afterwards.
 * @param sup Supplier of the resource that should be closed
 * @param op operation that should be performed on the resource before it is closed
 * @return The result of calling op.apply on the resource 
 */
public static <A extends AutoCloseable, B> B applyAndClose(Callable<A> sup, Function<A, B> op) {
    try (A res = sup.call()) {
        return op.apply(res);
    } catch (RuntimeException exc) {
        throw exc;
    } catch (Exception exc) {
        throw new RuntimeException("Wrapped in applyAndClose", exc);
    }
}

Other improvements

The java.io.File class is and related API is mostly obsolete. The standard way to work with files since Java 7 is the java.nio.file.Path and Files classes.
Use String.contains instead of indexOf.
Use a static import for toList: import static java.util.stream.Collectors.toList;.
It is probably only faster with a parallel stream if you're dealing with at least hundreds of files.

Using those your code would look like this:

List<Path> filteredList = targetList.stream()
    .filter(f -> Files.exists(f))
    .filter(f -> applyAndClose(
        () -> Files.lines(f),
        lines -> lines.anyMatch(line-> line.contains(","))))
    .collect(toList());

Thank for your kindness advices about saving the result and handling NotFound exception. These have already been handled before I delete some unconcerned codes for this issue. However, it seem not the best pratice to throw all exceptions to runtime. Handling with them in context maybe better. — TomX, Mar 08 '17 at 09:17
@卓建欢 You could add a third argument to `applyAndClose` of type `Consumer`, to act as an exception handler. — James_D, Mar 08 '17 at 12:41

score 1 · Answer 2 · answered Mar 08 '17 at 08:02

You can use the try-with-resources Statement:

The try-with-resources statement ensures that each resource is closed at the end of the statement.

targetList.parallelStream()
        .filter(f -> {
            try (BufferedReader br = new BufferedReader(new FileReader(f))) {
                return br.lines().anyMatch(line -> (line.indexOf(",") != -1));
            } catch (IOException e) {
                e.printStackTrace();
            }
            return false;
        })
        .collect(Collectors.toList());

score 1 · Answer 3 · answered Mar 08 '17 at 08:03

Use try with resources:

targetList.parallelStream()
        .filter(f -> {
                    try (BufferedReader br = new BufferedReader(new FileReader(f))) {
                        return br.lines().anyMatch(line -> (line.contains(",")));
                    } catch (IOException e) {
                        e.printStackTrace();
                    }
                    return false;
                }
        )
        .collect(Collectors.toList());

Kayaman · Answer 4 · 2017-03-08T08:40:54.883

You can completely drop the BufferedReader and FileReader (which shouldn't be used anyway) and use the following code (which assumes that the files are in UTF-8 encoding).

targetList.parallelStream()
        .filter(f -> {
            try {
                return Files.lines(f.toPath()).anyMatch(line -> line.contains(","));
            } catch(IOException e) {
                e.printStackTrace();
            }
            return false;
        }).collect(Collectors.toList());

You can't get rid of the try/catch without creating an additional wrapper that swallows the exception, like in Lii's answer.

Use lines() stream Lambda but need to close

4 Answers4

Other improvements