Text file manipulation with Java 8 and Guava

Question

I am trying to do some translation of text files. I have a translation .csv file and a file I need to translate (tab delimited). The translation is creating a file with an extra column of the translation.

I'd like to hear any suggestions on this.

import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
import com.google.common.collect.Lists;
import com.google.common.io.Files;

import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;


public class TextFileManipulationExample {

  public static void main(String[] args) {
    new TextFileManipulationExample().translate();
  }

  private void translate() {
    List<String> translationFileContent = readFile("dictionary.csv");
    Stream<List<String>> translationLines = translationFileContent.stream().map(
      line -> Splitter.on(",").splitToList(line));
    Map<List<String>, String> translationMap = translationLines.collect(Collectors.toMap(l -> Lists.newArrayList(l.subList(0, 2)), l -> l.get(2)));
    List<String> toTranslateLines = readFile("file_to_translate.txt");
    Stream<List<String>> listWithTranslation = toTranslateLines.stream().map(line -> {
      List<String> list = Lists.newArrayList(Splitter.on("\t").splitToList(line));
      if (translationMap.containsKey(list)) {
        list.add(translationMap.get(list));
      }
      return list;
    });
    String contentToWrite = listWithTranslation.map(lineList -> Joiner.on(",").join(lineList)).collect(
      Collectors.toList()).stream().reduce((t, u) -> t + "\n" + u).get();
    writeFile(contentToWrite, "translated.csv");
  }

  private void writeFile(String contentToWrite, String fileName) {
    try {
      Files.write(contentToWrite, new File(fileName), Charset.defaultCharset());
    } catch (IOException e) {
      throw new RuntimeException(e);
    }
  }

  private List<String> readFile(String fileName) {
    try {
      return Files.readLines(new File(fileName), Charset.defaultCharset());
    } catch (IOException e) {
      throw new RuntimeException(e);
    }
  }
}

Answer 1

You are using Java 8's functional stream programming, but you leave out method chaining. Depending on how you chain methods, it could be more or it could be less readable. You might be imagining this:

readFile("dictionary.csv").stream().map(line -> Splitter.on(",").splitToList(line)).collect(Collectors.toMap(l -> Lists.newArrayList(l.subList(0, 2)), l -> l.get(2)));

There is no way I could read that. However, consider this:

Map<List<String>, String> translationMap = readFile("dictionary.csv").stream()
            .map(line -> Splitter.on(",").splitToList(line))
            .collect(
                Collectors.toMap(
                    l -> Lists.newArrayList(l.subList(0, 2)),
                    l -> l.get(2)
                )
            );

I find this much more readable. By simply reading vertically, I can determine that the code does this:

1. Reads dictionary.csv into a list of strings (via readFile).
2. Split each string in the dictionary on ",", into a List<String>.
3. Collect the List<String>s into a Map, where
   1. The keys are the first two elements of each list, still in a List
   2. The values are the 3rd element of each list.

On the other hand, your code as is is littered with type names; do I really need to know that that is a Stream<List<String>> when that's easily determined from context?:

List<String> translationFileContent = readFile("dictionary.csv");
Stream<List<String>> translationLines = translationFileContent.stream().map(
  line -> Splitter.on(",").splitToList(line));
Map<List<String>, String> translationMap = translationLines.collect(Collectors.toMap(l -> Lists.newArrayList(l.subList(0, 2)), l -> l.get(2)));

---

Alternatively, if you really like the temporary variables, put some empty lines in there to separate logical sections. Also, consider using Project Lombok, where you could just do this:

List<String> translationFileContent = readFile("dictionary.csv");
val translationLines = translationFileContent.stream().map(
    line -> Splitter.on(",").splitToList(line));
val translationMap = translationLines.collect(Collectors.toMap(l -> Lists.newArrayList(l.subList(0, 2)), l -> l.get(2)));

(Coder discretion advised on where you should use val).

Answer 2

Chaining

Avoid temporary variables and just chain all the methods, it is common practice in FP.

Utility class

Java forces you to catch some exceptions, you may want to read files without that tedious try catch other times, so I suggest a FileHandler class to contain your readfile and writefile.

Arguments

The translate method should take input and output filenames as arguments.

Answer 3

You want to remove the function chaining and temporary variables by replacing it with other functions. I strongly recommend against this.

Breaking this into small methods, as you suggest, isn't good. Functional programming is designed to be highly readable as is like this. Hiding the details behind functions makes it hard to understand:

Map<List<String>, String> translationMap = convertTranslationLinesToMap(
    splitEachLineByComma(readFile("dictionary.csv"))
);

And each of those functions would be trivial one-two lines of code. That's not what functions are for.

Your IDE should format your code with newlines and indents, or leave the code unformatted when it is wrapped across lines. Making individual functions to do this would be the analogue of making individual functions the following pre-Java 8 code:

List<String> translationFileContent = readFile("dictionary.csv");
Map<List<String>, String> translationMap = new HashMap<>();

for (String line : translationFileContent) {
    List<String> list = Splitter.on(",").splitToList(line);
    translationMap.put(Lists.newArrayList(list.subList(0, 2)), list.get(2));
}

This code should not be moved into its own function as it's highly specific to this location and refactoring into a function just seems useless. Don't do this for the Java 8 code. In all honesty, separating each call into temporary variables is probably cleaner than making new functions for everything.

Additionally, formatting the Java 8 code across multiple lines isn't "relying on indentation and newlines" any more than the other code example is. Try reading this:

List<String> translationFileContent = readFile("dictionary.csv"); Map<List<String>, String> translationMap = new HashMap<>(); for (String line : translationFileContent) { List<String> list = Splitter.on(",").splitToList(line); translationMap.put(Lists.newArrayList(list.subList(0, 2)), list.get(2)); }

Similarly, Java 8 code is not meant to be formatted like that.

Regarding the concern of colleagues formatting away the line breaks, you could add a comment explaining why it is like that. But poorly chosen IDE format settings could still interfere, unless you did something like this (this isn't good in practice, though):

Map<List<String>, String> translationMap = readFile("dictionary.csv").stream()
                                                              // For each line,
            .map(line -> Splitter.on(",").splitToList(line))  // split on commas, then
            .collect(                                         // collect the result
                Collectors.toMap(                             // into a map
                    l -> Lists.newArrayList(l.subList(0, 2)), // whose keys are the first two elements of each line
                    l -> l.get(2)                             // and values are the third.
                )
            );

This isn't a good idea in general; we shouldn't need comments at the end of each line.