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 an intention is to create small reusable readable functions. But functions that operate on global mutable states are usually not reusable and lead to unexpected bugs.

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 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) {
        Columns header = new Columns("One", "Two", "Three", "Four");
        header.addLine("10000000", "20000000", "30000000", "40000000").print();
        header.addLine("1", "2", "3", "4").print();
     }
}

Preference

I also prefer Listing A to Listing B. Reading the latter code requires a lot of jumping around. The helper methods are procedures that interact by reading and setting instance variables as side-effects, making it even harder to follow.

If the helpers were functions rather than procedures, for example:

this.columnWidths = adjustColumnWidths(this.columnWidths, line);

… then there might be some benefit to that approach. But in this case, the whole addLine() is simple enough that it would be better not to split it up.

If you want to improve the readability, you could just add some comments.

I wouldn't bother to make this class thread-safe. If it is used by multi-theaded code, then the caller can take the responsibility to ensure mutual exclusion. That way, single-threaded code would not incur locking overhead.

Critique

You have severe inefficiencies since you are using += everywhere to concatenate strings. Repeated string concatenation should be done using StringBuilder. You can even specify exactly how large the buffer needs to be before starting to build the string.

The instance variables should be private. Default access is rarely the most appropriate.

The initial value int numColumns = -1 is weird. Why not just leave maxLengths null to indicate that no lines have been added yet? I also think that widths would be a slightly better name. Furthermore, since the number of columns should never change after the first line, you don't need an ArrayList<Integer> — an array would do just fine.

Similarly, lines could just be a List<String[]>.

The fact that print() calls System.out.println() is weird. It causes the output to be terminated by two newlines. Arguably, one newline, or even no newline, would be appropriate.

The pad() helper function could be static, and therefore should be static.

The IllegalArgumentException should have a more informative message.

Suggested solution

import java.util.Arrays;
import java.util.ArrayList;
import java.util.List;  

public class Columns {

    private List<String[]> lines = new ArrayList<>();
    private int[] widths;
    private int lineLength;

    public Columns addLine(String... line) {
        // Learn or check the number of columns
        if (this.widths == null) {
            this.widths = new int[line.length];
        } else if (this.widths.length != line.length) {
            throw new IllegalArgumentException(
                "Expected " + this.widths.length + " columns, got " +
                line.length + " columns");
        }

        // Expand column widths as necessary
        this.lineLength = 0;
        for (int i = 0; i < line.length; i++) {
            this.widths[i] = Math.max(this.widths[i], line[i].length() + 1);
            this.lineLength += this.widths[i];
        }

        lines.add(Arrays.copyOf(line, line.length));
        return this;
    }

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

    public String toString() {
        StringBuilder result = new StringBuilder(
            (lineLength + System.lineSeparator().length()) * lines.size()
        );

        for (String[] line : lines) {
            for (int i = 0; i < widths.length; i++) {
                result.append(line[i]);
                pad(result, widths[i] - line[i].length());
            }
            result.append(System.lineSeparator());
        }
        return result.toString();
    }

    private static void pad(StringBuilder sb, int spaces) {
        for (int i = 0; i < spaces; i++) {
            sb.append(" ");
        }
    }

}