HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Combining contents of files by reading them in a human ordering

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
readinghumancombiningcontentsorderingfilesthem

Problem

This code review is inspired by the this StackOverflow question and written in Java 8.

The code is supposed to:

  • Order all files, with syntax jobXXX.script, where XXX is a number, by the number.



  • Write the contents of the files in that ordering to one file.



I am asking for a general code review and there are two things that are bugging me:

  • I need to wrap Exceptions inside lambda expressions.



  • The file that is being written to, has an imperfect unnecessary annoying OCD-triggering newline at the end.



```
package testproject4;

import java.io.BufferedWriter;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Comparator;
import java.util.stream.Stream;

/**
*
* @author Beheerder
*/
public class TestProject4 {
private void init() throws IOException {
Path directory = Paths.get("C:\\Users\\Frank\\Downloads\\testjob");
try (BufferedWriter writer = Files.newBufferedWriter(directory.resolve("masterjob.script"))) {
Files.list(directory)
.filter(path -> Files.isRegularFile(path))
.filter(path -> path.getFileName().toString().matches("job\\d+.script"))
.sorted(Comparator.comparingInt(this::pathToInt))
.flatMap(this::wrappedLines)
.forEach(string -> wrappedWrite(writer, string));
}
}

private int pathToInt(final Path path) {
return Integer.parseInt(path.getFileName()
.toString()
.replaceAll("job(\\d+).script", "$1")
);
}

private Stream wrappedLines(final Path path) {
try {
return Files.lines(path);
} catch (IOException ex) {
//swallow
return null;
}
}

private void wrappedWrite(final BufferedWriter writer, final String string) {
try {
writer.write(string);
writer.newLine();
}

Solution

NewLine

The newline at the end is correct... at least according to your code.

Files.lines(...) ignores the last line of input files if it has no characters. This is a standard system to have for most operating systems (the last character in a text file is a newline).

Your code is perpetuating this process, and it always adds a newline after each line printed.

Thus, your file always ends up with a newline at the end. This is a good thing.

Exceptions

Java Lambdas cannot contain expressions that throw caught exceptions. This is a weakness. The weakness is so sever that as part of Java8, there is the new Exception UncheckedIOException so that IO-based operations can throw an unchecked exception.

The easiest way to use it is to change your catch blocks to:

private Stream wrappedLines(final Path path) {
    try {
        return Files.lines(path);
    } catch (IOException ioe) {
        throw new UncheckedIOException(ioe);
    }
}


The alternative is to wrap the exception inside the lambda with:

try (BufferedWriter writer = Files.newBufferedWriter(directory.resolve("masterjob.script"))) {
        Files.list(directory)
                .filter(path -> Files.isRegularFile(path))
                .filter(path -> path.getFileName().toString().matches("job\\d+.script"))
                .sorted(Comparator.comparingInt(this::pathToInt))
                .flatMap(try { this::wrappedLines } catch(IOException ioe) {throw new UncheckedIOException(ioe)})
                .forEach(string -> try {wrappedWrite(writer, string)} catch(IOException ioe) {throw new UncheckedIOException(ioe)});
    }


General

The Java8 side of this otherwise looks OK, and works fine for me too.

Code Snippets

private Stream<String> wrappedLines(final Path path) {
    try {
        return Files.lines(path);
    } catch (IOException ioe) {
        throw new UncheckedIOException(ioe);
    }
}
try (BufferedWriter writer = Files.newBufferedWriter(directory.resolve("masterjob.script"))) {
        Files.list(directory)
                .filter(path -> Files.isRegularFile(path))
                .filter(path -> path.getFileName().toString().matches("job\\d+.script"))
                .sorted(Comparator.comparingInt(this::pathToInt))
                .flatMap(try { this::wrappedLines } catch(IOException ioe) {throw new UncheckedIOException(ioe)})
                .forEach(string -> try {wrappedWrite(writer, string)} catch(IOException ioe) {throw new UncheckedIOException(ioe)});
    }

Context

StackExchange Code Review Q#44392, answer score: 2

Revisions (0)

No revisions yet.