Table printing code using a fluent interface

Let's say you want to display something like:

One Two Three Four  1   2   3     4     

And since you hate the idea of predefining the widths of the columns you want to print it with something like this:

public static void main(String[] args) {     new Columns()        .addLine("One", "Two", "Three", "Four")        .addLine("1", "2", "3", "4")        .print()        ; } 

Either listing below can do this. Which is easier to read? How would you improve either of them? Do either paint us into a corner? Any multithreaded concerns? How could it be made extensible? Would the getThis() trick help here or is it a bad idea?

Listing A

import java.util.ArrayList; import java.util.Arrays; import java.util.List;    public class Columns {      List<List<String>> lines = new ArrayList<>();     List<Integer> maxLengths = new ArrayList<>();     int numColumns = -1;      public Columns addLine(String... line) {          if (numColumns == -1){             numColumns = line.length;             for(int i = 0; i < numColumns; i++) {                 maxLengths.add(0);             }         }          if (numColumns != line.length) {             throw new IllegalArgumentException();         }          for(int i = 0; i < numColumns; i++) {             maxLengths.set(  i, Math.max( maxLengths.get(i), line[i].length() )  );         }          lines.add( Arrays.asList(line) );          return this;     }      public void print(){         System.out.println( toString() );     }      public String toString(){         String result = "";         for(List<String> line : lines) {             for(int i = 0; i < numColumns; i++) {                 result += pad( line.get(i), maxLengths.get(i) + 1 );                             }             result += System.lineSeparator();         }         return result;     }      private String pad(String word, int newLength){         while (word.length() < newLength) {             word += " ";                     }                return word;     } } 

Listing B

import java.util.ArrayList; import java.util.Arrays; import java.util.List;  public class Columns {      List<List<String>> lines = new ArrayList<>();     List<Integer> maxLengths = new ArrayList<>();     int numColumns = -1;     String[] line;      public Columns addLine(String... line) {          this.line = line;         learnNumOfColumnsOnce();         ensureConsistantNumOfColumns();         widenColumnsThatNeedIt();         rememberLine();         return this;     }      private void learnNumOfColumnsOnce() {         if (numColumns == -1){             numColumns = line.length;             for(int i = 0; i < numColumns; i++) {                 maxLengths.add(0);             }         }     }      private void ensureConsistantNumOfColumns() {         if (numColumns != line.length) {             throw new IllegalArgumentException();         }     }      private void widenColumnsThatNeedIt() {         for(int i = 0; i < numColumns; i++) {             maxLengths.set(  i, Math.max( maxLengths.get(i), line[i].length() )  );         }     }      private void rememberLine() {         lines.add( Arrays.asList(line) );     }      public void print(){         System.out.println( toString() );     }      public String toString(){         String result = "";         for(List<String> line : lines) {             for(int i = 0; i < numColumns; i++) {                 result += padWithSpaces( line.get(i), maxLengths.get(i) + 1 );             }             result += System.lineSeparator();         }         return result;     }      private String padWithSpaces(String word, int newLength){         while (word.length() < newLength) {             word += " ";         }         return word;     } } 

Replay

Which is easier to read? How would you improve either of them? Do either paint us into a corner?

I love A. It is simple, clear, concise and meaningful. B is the same as A but with one major difference: the method addLine was cut into several small pieces.

In my opinion, the issue with B is that it extracts from one meaningful method (adding a line) a lot of small awkward methods, and in the process completely obscuring the real task at play: none of those methods are really reusable and probably none of those methods would make sense in a context other than addLine (think removeLine, withColor, withFont...).

Think back and focus on what you want the code to do. We have addLine and we expect it to add a line to our columns. Clearly, there are several code paths to deal with: notably, the first case is to determine the length of each line, etc. But a method can have 30 lines of code without it doing too much work, because it wouldn't make sense to cut that work in pieces: they are all part of the same unit of work. All of the pieces are needed to make the lot work, and they need to be called and assembled in a specific way that is unique to adding a line. In that sense, having multiple methods may be fragile because they are coupled to each-other.

Of course, if that work would require lots of lines of code, you can move part of in a dedicated method. Additionally, work that requires few lines of code could also be extracted in multiple utility methods.

The point is to find a balance between small, easy to read methods and logical unit of work. Part of it can also rely on opinions — both approaches are correct and none of them are fundamentally bad.

Any multithreaded concerns?

Both approaches won't work in a multi-threaded environment because Columns isn't thread-safe: it has state, for example with List<List<String>> lines and List<Integer> maxLengths. One possible way to make it thread-safe would be to return a new Columns instance each time addLine is called (for that, we can add a private constructor taking lines, maxLengths and numColumns), and making Columns immutable in the process (do not update the fields of this but use the new values to construct the new instance and return it).

How could it be made extensible?

It is already extensible enough. The data structures for Columns are kept hidden; I can see a removeLine, withColor (setting a color for each column), withFont (setting a font for each column) added without refactoring the current code.

Would the getThis() trick help here or is it a bad idea?

Do you really intend to have classes inhering from Columns? Keep it simple, you most likely aren't going to need it.



Other comments on the code, applicable to both examples:

  • You are throwing an IllegalArgumentException in case a line with a different length is added:
    throw new IllegalArgumentException();
    
    

    That's great, but consider adding a message so that it is clearer for the user what went wrong. IllegalArgumentException with no message is not really helpful.

  • Both in toString and in pad, you are concatenating String with the += operator. This can lean to serious performance issues. When used inside a loop, you should always use a StringBuilder (or a StringJoiner).

Which is easier to read?

I prefer first Listing over second. Second one reads very procedural. It adds unnecessary field line that is just a global variable for procedures to read. I understand that in second listing is an intention to create small reusable more readable functions. But functions that operate on global mutable states are not reusable.

Any multithreaded concerns? How could it be made extensible?

Your solution is good for usage in single place for fluent API. But as soon as you start passing this builder to other objects you can face several potential problems because of mutability.

  • multiple thread adding line could break consistency of data by overriding calculations of each other
  • multiple clients adding lines affects each other instances, so there is no way to initialize one builder and create 2 separate results. As an example could be one builder with initialized header that is reused several times for different table data.


Here I've tried to demonstrate here how to make Columns immutable. Also tried to extract utility functions that makes code readable and don't mutate global state.

import java.util.ArrayList;
import java.util.List;
import java.util.function.BiFunction;
import java.util.stream.Stream;

import static java.lang.Math.min;
import static java.lang.System.lineSeparator;
import static java.util.Arrays.asList;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;

public class Columns {

    //fields are final
    private final List<List<String>> lines;
    private final List<Integer> lengths;

    private Columns(List<List<String>> lines, List<Integer> lengths) {
        this.lines = lines;
        this.lengths = lengths;
    }

    //this constructor just for fluency
    public Columns(String... line) {
        this(asList(asList(line)), lengths(line));
    }

    //returns new instance without modifying current
    public Columns addLine(String... line) {
        ensure(lengths.size() != line.length, "Wrong number of columns");

        List<List<String>> lines = append(this.lines, asList(line));
        List<Integer> lengths = merge(Math::max, this.lengths, lengths(line)); //new length is max between previous and new
        return new Columns(lines, lengths);
    }

    private static void ensure(boolean valid, String errorMessage) throws IllegalArgumentException {
        if (!valid) {
            throw new IllegalArgumentException(errorMessage);
        }
    }

    //a helper methods create new list with appended value
    private static <V> List<V> append(List<V> values, V value) {
        List<V> result = new ArrayList<>(values);
        result.add(value);
        return result;
    }

    //calculates length of each string
    private static List<Integer> lengths(String[] line) {
        return Stream.of(line).map(String::length).collect(toList());
    }

    //merges 2 lists applying a function to elements pairwise
    private static <L, R, M> List<M> merge(BiFunction<L, R, M> merge, List<L> left, List<R> right) {
        int length = min(left.size(), right.size());
        List<M> result = new ArrayList<>(length);
        for (int i = 0; i < length; i++) {
            result.add(merge.apply(left.get(i), right.get(i)));
        }
        return result;
    }

    public void print() {
        System.out.println(toString());
    }

    public String toString() {
        return lines.stream()
                .map(line -> merge(this::pad, line, lengths))       //pad columns
                .map(line -> line.stream().collect(joining(" ")))   //join columns into line by space
                .collect(joining(lineSeparator()));                 //join lines by lineSeparator
    }

    private String pad(String word, int newLength) {
        StringBuilder builder = new StringBuilder(word);
        while (builder.length() < newLength) {
            builder.append(" ");
        }
        return builder.toString();
    }

    public static void main(String[] args) {
        new Columns("One", "Two", "Three", "Four")
                .addLine("1", "2", "3", "4")
                .print();
    }
}

Category: java Time: 2016-07-29 Views: 0

Related post

iOS development

Android development

Python development

JAVA development

Development language

PHP development

Ruby development

search

Front-end development

Database

development tools

Open Platform

Javascript development

.NET development

cloud computing

server

Copyright (C) avrocks.com, All Rights Reserved.

processed in 0.145 (s). 12 q(s)