1

Given the following code, how can I simplify it to a single, functional line?

    // DELETE CSV TEMP FILES
    final Map<Boolean, List<File>> deleteResults = Stream.of(tmpDir.listFiles())
            .filter(tempFile -> tempFile.getName().endsWith(".csv"))
            .collect(Collectors.partitioningBy(File::delete));

    // LOG SUCCESSES AND FAILURES
    deleteResults.entrySet().forEach(entry -> {
        if (entry.getKey() && !entry.getValue().isEmpty()) {
            LOGGER.debug("deleted temporary files, {}",
                    entry.getValue().stream().map(File::getAbsolutePath).collect(Collectors.joining(",")));
        } else if (!entry.getValue().isEmpty()) {
            LOGGER.debug("failed to delete temporary files, {}",
                    entry.getValue().stream().map(File::getAbsolutePath).collect(Collectors.joining(",")));
        }
    });

This is a common pattern I run into, where I have a stream of things, and I want to filter this stream, creating two streams based off that filter, where I can then do one thing to Stream A and another thing to Stream B. Is this an anti-pattern, or is it supported somehow?

Jacob G.
  • 28,856
  • 5
  • 62
  • 116
liltitus27
  • 1,670
  • 5
  • 29
  • 46
  • 2
    Use `map`. Or, if you really want, use `partition`. – Boris the Spider Dec 15 '17 at 19:25
  • 1
    Possible duplicate of [Can you split a stream into two streams?](https://stackoverflow.com/questions/19940319/can-you-split-a-stream-into-two-streams) – Alex Savitsky Dec 15 '17 at 19:30
  • you are creating two streams *not* by filtering, but by grouping, in your case by partitioning; you can't really split a stream in two - you would have to collect in two parts; this is not an anti-pattern and the answer shown here does exactly that – Eugene Dec 17 '17 at 21:10

2 Answers2

5

If you particularly don't want the explicit variable referencing the interim map then you can just chain the operations:

.collect(Collectors.partitioningBy(File::delete))
.forEach((del, files) -> {
    if (del) {
        LOGGER.debug(... files.stream()...);
    } else {
        LOGGER.debug(... files.stream()...);
    });
sprinter
  • 27,148
  • 6
  • 47
  • 78
3

If you want to log all files of the either category together, there is no way around collecting them into a data structure holding them, until all elements are known. Still, you can simplify your code:

Stream.of(tmpDir.listFiles())
      .filter(tempFile -> tempFile.getName().endsWith(".csv"))
      .collect(Collectors.partitioningBy(File::delete,
          Collectors.mapping(File::getAbsolutePath, Collectors.joining(","))))
.forEach((success, files) -> {
    if (!files.isEmpty()) {
        LOGGER.debug(success? "deleted temporary files, {}":
                              "failed to delete temporary files, {}",
                     files);
    }
});

This doesn’t collect the files into a List but into the intended String for the subsequent logging action in the first place. The logging action also is identical for both cases, but only differs in the message.

Still, the most interesting thing is why deleting a file failed, which a boolean doesn’t tell. Since Java 7, the nio package provides a better alternative:

Create helper method

public static String deleteWithReason(Path p) {
    String problem;
    IOException ioEx;

    try {
        Files.delete(p);
        return "";
    }
    catch(FileSystemException ex) {
        problem = ex.getReason();
        ioEx = ex;
    }
    catch(IOException ex) {
        ioEx = ex;
        problem = null;
    }
    return problem!=null? problem.replaceAll("\\.?\\R", ""): ioEx.getClass().getName();
}

and use it like

Files.list(tmpDir.toPath())
      .filter(tempFile -> tempFile.getFileName().toString().endsWith(".csv"))
      .collect(Collectors.groupingBy(YourClass::deleteWithReason,
          Collectors.mapping(p -> p.toAbsolutePath().toString(), Collectors.joining(","))))
.forEach((failure, files) -> 
    LOGGER.debug(failure.isEmpty()? "deleted temporary files, {}":
                           "failed to delete temporary files, "+failure+ ", {}",
                 files)
);

The disadvantage, if you want to call it that way, is does not produce a single entry for all failed files, if they have different failure reasons. But that’s obviously unavoidable if you want to log them with the reason why they couldn’t be deleted.

Note that if you want to exclude “being deleted by someone else concurrently” from the failures, you can simply use Files.deleteIfExists(p) instead of Files.delete(p) and being already deleted will be treated as success.

Holger
  • 285,553
  • 42
  • 434
  • 765