Print columns of text without hard coding the width

Question

If you'd like to print this:

One Two Three Four
 1   2    3    4  

using this:

new Columns()
    .withColumnSeparator(" ")
    .withPad(" ")
    .alignCenter()
    .addLine("One", "Two", "Three", "Four")
    .addLine("1", "2", "3", "4")
    .print()
;

all you need is this:

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

public class Columns {

    List<List<String>> lines = new ArrayList<>();
    List<Integer> maxLengths = new ArrayList<>();
    int numColumns;
    int oldLength;
    boolean firstCall = true;
    String columnSeparator = " ";
    String pad = " ";
    BiFunction<String, String, String> align = (word, pad) -> word += pad;

    public Columns addLine(String... line) {

        if (firstCall){
            for(int column = 0; column < line.length; column++) {
                maxLengths.add(0);
            }
            oldLength = line.length;
        }
        
        if (oldLength != line.length) {
            throw new IllegalArgumentException();
        }

        for(int column = 0; column < line.length; column++) {
            int length = Math
                .max( 
                    maxLengths.get(column), 
                    line[column].length() 
                )
            ;
            maxLengths.set( column, length );
        }

        lines.add( Arrays.asList(line) );
        firstCall = false;
        return this;
    }

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

    public String toString(){
        String result = "";
        for(List<String> line : lines) {
            for(int column = 0; column < line.size(); column++) {
                result += pad( line.get(column), maxLengths.get(column) );
                if (column < line.size() - 1) { 
                    result += columnSeparator; 
                }
            }
            result += System.lineSeparator();
        }
        return result;
    }

    private String pad(String word, int newLength){
        while (word.length() < newLength) {
            word = align.apply(word, pad);
        }       
        return word;
    }

    public Columns withColumnSeparator(String columnSeparator){
        this.columnSeparator = columnSeparator;
        return this;
    }
        
    public Columns withPad(String pad){
        this.pad = pad;
        return this;
    }
    
    public Columns alignLeft(){
        align = (word, pad) -> word = word + pad;
        return this;
    }

    public Columns alignRight(){
        align = (word, pad) -> word = pad + word;
        return this;
    }

    public Columns alignCenter(){
        align = (word, pad) -> {
            return (word.length() % 2 == 0) 
                ? pad + word
                : word + pad
            ;
        };
        return this;
    }
}

By waiting until all the lines have been added before printing it can figure out the width each column needs.

Looking for a code review to reveal problems. Anything from better names to fewer bugs to better ideas.

Answer 1

The idea is good, and on the first look I don't see any bugs, but the implementation is a bit strange.

---

All fields are lacking a private modifier.

---

Some of the lambdas have an unnessecary assignment:

(word, pad) -> word = word + pad
(word, pad) -> word += pad

The assignment to the parameter word does nothing. Just

(word, pad) -> word + pad

is needed. You do it correctly in the "center" lamdba.

---

Also assigning "new" lambdas everywhere isn't very optimal. The lambdas should either be static constants or private methods:

private static final BiFunction<String, String, String> ALIGN_RIGHT = (word, pad) -> word + pad;

private BiFunction<String, String, String> align = ALIGN_RIGHT;

public Columns alignRight() {
    align = ALIGN_RIGHT;
    return this;
}

or

private BiFunction<String, String, String> = Columns::doAlignRight;

// TODO `doAlignRight` isn't a good name. Find better.
private static String doAlignRight(String word, String pad) {
    return word + pad;
}

public Columns alignRight() {
    align = Columns::doAlignRight;
    return this;
}

EDIT: In Java lambdas are just to short way to implement and instantiate an interface. For example:

align = (word, pad) -> word + pad;

in the method alignRight() is short for

align = new BiFunction<String, String, String>() {
    public String apply(String word, String pad) {
        return word + pad;
    }
};

In other words, everytime one one of the align... methods is called a new object is created. However this is unnessecary, because the objects are the same every time. Instead by, for example, keeping a single instance in a static variable and reusing them, much less memory management is needed.

---

Method names could be a bit better.

with... usually implies that a new instance (of a usually immutable class) is returned. set... would be better.

The align...() methods should also start with set... to match the other configuration methods.

It can be confusing that there is both a field and a method called pad.

---

The field numColumns is not used. firstCall and oldLength isn't really needed. Instead of firstCall it could be checked if lines is empty, and when not use the length of one of its entries instead of oldLength.

---

String concatenation shouldn't be used in loops. A StringBuilder would be a better choice.