11
\$\begingroup\$

I wrote a class which can format input so that it is properly aligned.

I'm interested in all suggestions (regarding structure, usability, style, naming, comments, etc).

I'm especially wondering about:

  • Is there a better way to write the tests? I want to first test the Content class, and then test the Align class (so that when an error is thrown, I can see right away where it is). Right now, I'm basically just copy-pasting the Content tests (the add and set tests), and adding align.output((String s) -> nothing());. This doesn't seem like the right way to do it.

Example Usage:

   public static void main(String[] args) {
       Align align = new Align();
       // header
       align.addLine("Name", "Age", "Address", "Phone");

       // data
       align.addLine("Alice", "32", "United States", "555-123456");
       align.addLine("Bob", "12", "nowhere", "555-123456");
       align.addLine("Carol", "78", "1080 Maple Court Cape Girardeau, MO 63701", "555-123456");
       align.addLine("Ed", "159", "Fake Stree, Fakesville");

       // output
        align.output((String s) -> System.out.println(s));

       // add a forgotten column
       align.addColumn(3, "Hair Color", "Red", "Brown", "Blue", "Purple", "Orange");
       align.output((String s) -> System.out.println(s));
    }

Output:

Name     Age     Address                                       Phone     
Alice    32      United States                                 555-123456
Bob      12      nowhere                                       555-123456
Carol    78      1080 Maple Court Cape Girardeau, MO 63701     555-123456
Ed       159     Fake Stree, Fakesville                        -         

Name     Age     Address                                       Hair Color     Phone     
Alice    32      United States                                 Red            555-123456
Bob      12      nowhere                                       Brown          555-123456
Carol    78      1080 Maple Court Cape Girardeau, MO 63701     Blue           555-123456
Ed       159     Fake Stree, Fakesville                        Purple         -         
-        -       -                                             Orange         -  

Align Class:

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

public class Align {

    private String paddingChar = "-";
    private int minSpacing = 5;

    protected Content content;

    public Align() {
        content = new Content();
    }

    /**
     * set the character that is used for values that don't exist.
     *
     * @param paddingChar padding char
     */
    public void setPaddingChar(String paddingChar) {
        this.paddingChar = paddingChar;
    }

    /**
     * the minimum amount of spaces between columns in the formated output.
     *
     * Must be at least 0, but a minimum of at least 1 is suggested.
     *
     * @param minSpacing minimum of spaces (must be at least 0)
     */
    public void setMinSpacing(int minSpacing) {
        if (minSpacing < 0) {
            throw new IllegalArgumentException("min spacing must be at least 0");
        }

        this.minSpacing = minSpacing;
    }

    public void addLine(int position, List<String> lineContent) {
        content.addLine(position, lineContent);
    }

    public void addLine(List<String> lineContent) {
        addLine(content.lineCount(), lineContent);
    }

    public void addLine(String... lineContent) {
        addLine(content.lineCount(), new ArrayList<>(Arrays.asList(lineContent)));
    }

    public void addLine(int position, String... lineContent) {
        addLine(position, new ArrayList<>(Arrays.asList(lineContent)));
    }

    public void addColumn(int position, List<String> columnContent) {
        content.addColumn(position, columnContent);
    }

    public void addColumn(List<String> columnContent) {
        addColumn(content.columnCount(), columnContent);
    }

    public void addColumn(String... columnContent) {
        addColumn(content.columnCount(), new ArrayList<>(Arrays.asList(columnContent)));
    }

    public void addColumn(int position, String... columnContent) {
        addColumn(position, new ArrayList<>(Arrays.asList(columnContent)));
    }

    /**
     * sets the value at the given column and row to the given content.
     *
     * If the value doesn't exist already, an IllegalArgumentException will be
     * thrown.
     *
     * @param column column
     * @param row row
     * @param value value
     */
    public void set(int column, int row, String value) {
        content.set(column, row, value);
    }

    /**
     * passes the formated string to consumer.
     *
     * To get the formated string, use toString.
     *
     * @param consumer consumer
     */
    public void output(Consumer<String> consumer) {
        consumer.accept(toString());
    }

    @Override
    public String toString() {
        List<Integer> maxContentLength = getMaxLengths();
        List<String> spaces = getSpaces(maxContentLength);

        StringBuilder out = new StringBuilder();
        for (Content.Line line : content.lines) {
            for (int columnIndex = 0; columnIndex < line.size(); columnIndex++) {
                String column = line.get(columnIndex);
                out.append(column);
                out.append(spaces.get(columnIndex)
                        .substring(0,
                                spaces.get(columnIndex).length()
                                - column.length())); // align
            }
            out.append("\n");
        }
        return out.toString();
    }

    /**
     * returns a list of spaces of the given lengths.
     *
     * @param lengths lengths
     * @return list of spaces
     */
    private List<String> getSpaces(List<Integer> lengths) {
        List<String> spaces = new ArrayList<>();
        for (Integer length : lengths) {
            spaces.add(getSpaces(length));
        }
        return spaces;
    }

    /**
     * returns n spaces.
     *
     * @param n number of spaces
     * @return n spaces
     */
    private static String getSpaces(int n) {
        StringBuilder spaces = new StringBuilder(n);
        for (int i = 0; i < n; i++) {
            spaces.append(" ");
        }
        return spaces.toString();
    }

    /**
     * returns a list containing the length of the longest string of each
     * column.
     *
     * @return lengths
     */
    private List<Integer> getMaxLengths() {
        List<Integer> maxContentLength = new ArrayList<>();
        for (int i = 0; i <= content.columnCount(); i++) {
            maxContentLength.add(0); // init max length with 0
        }

        for (Content.Line line : content.lines) {
            for (int columnIndex = 0; columnIndex < content.columnCount(); columnIndex++) {
                String column = line.get(columnIndex);
                if (column.length() > maxContentLength.get(columnIndex)) {
                    maxContentLength.set(columnIndex, column.length() + minSpacing);
                }
            }
        }
        return maxContentLength;
    }

    protected class Content {

        private List<Line> lines;

        public Content() {
            lines = new ArrayList<>();
        }

        /**
         * returns number of lines.
         *
         * @return number of lines
         */
        public int lineCount() {
            return lines.size();
        }

        /**
         * returns number of columns.
         *
         * @return number of columns
         */
        public int columnCount() {
            if (lines.isEmpty()) {
                return 0;
            }
            return lines.get(0).size(); // all lines are the same length
        }

        public void set(int column, int row, String value) {
            if (row < 0 || row > lineCount()) {
                throw new IllegalArgumentException("row doesn't exist");
            }
            Line line = lines.get(row);
            if (column < 0 || column > columnCount()) {
                throw new IllegalArgumentException("column doesn't exist");
            }
            line.set(column, value);
        }

        public void addColumn(int position, List<String> columnContent) {
            if (position < 0) {
                throw new IllegalArgumentException("Column position is negative");
            }

            // if the new position is outside the current content, extend short lines to position
            if (position > columnCount()) {
                for (Line line : lines) {
                    padTo(line, position);
                }
            }

            // add new lines for content of this column in case the column is too long
            while (lines.size() < columnContent.size()) {
                Line newLine = new Line(new ArrayList<>());
                padTo(newLine, position); // if the current line is new, and thus too short, pad it
                lines.add(newLine);
            }

            for (int i = 0; i < columnContent.size(); i++) {
                Line line = lines.get(i);
                line.add(position, columnContent.get(i)); // add actual content of column
            }

            if (columnContent.size() > columnCount()) {
                for (Line line : lines) {
                    padTo(line, content.columnCount());
                }
            }

            // if the column was shorter then previous once, shift the rest of the lines and insert padding char for the new column
            if (columnContent.size() < lines.size()) {
                shift(position, lines.subList(columnContent.size(), lines.size()));
            }
        }

        public void addLine(int position, List<String> lineContent) {
            if (position < 0) {
                throw new IllegalArgumentException("Line position is negative");
            }

            // add elements to this line if it is too short
            padTo(lineContent, columnCount());

            if (lineContent.size() > columnCount()) { // add elements to all other lines if this line is the new longest line
                for (Line line : lines) {
                    padTo(line, lineContent.size()); // padd to new longest line
                }
            }

            while (lines.size() < position) { // add new empty lines if this line is outside field
                List<String> emptyLineContent = new ArrayList<>();
                for (int i = 0; i < columnCount(); i++) {
                    emptyLineContent.add(paddingChar);
                }
                lines.add(new Line(emptyLineContent));
            }
            lines.add(position, new Line(lineContent)); // add actual line
        }

        /**
         * shifts all given lines at given position one entry to the right.
         *
         * The padding character will be inserted at the new position.
         *
         * @param position position
         * @param list list
         */
        private void shift(int position, List<Line> list) {
            for (Line line : list) {
                line.add(position, paddingChar);
            }
        }

        /**
         * add elements to given line until its length is the given max.
         *
         * @param line line
         * @param max max
         */
        private void padTo(List<String> line, int max) {
            while (line.size() < max) {
                line.add(paddingChar);
            }
        }

        private class Line extends ArrayList<String> {
            public Line(List<String> line) {
                super(line);
            }
        }
    }
}

Tests:

import org.junit.Test;

public class AlignTest {

    private static Align getAlign(int columns, int rows) {
        Align align = new Align();
        for (int i = 0; i < rows; i++) {
            String[] row = new String[columns];
            for (int j = 0; j < columns; j++) {
                row[j] = String.valueOf(i) + String.valueOf(j);
            }
            align.addLine(row);
        }
        return align;
    }

    /** TEST CONTENT **/

    @Test(expected=IllegalArgumentException.class)
    public void setNegativeColumn() {
        getAlign(2, 3).set(-1, 1, "new value");
    }

    @Test(expected=IllegalArgumentException.class)
    public void setNonExistingColumn() {
        getAlign(2, 3).set(20, 1, "new value");
    }

    @Test(expected=IllegalArgumentException.class)
    public void setNegativeLine() {
        getAlign(2, 3).set(1, -1, "new value");
    }

    @Test(expected=IllegalArgumentException.class)
    public void setNonExistingLine() {
        getAlign(2, 3).set(1, 20, "new value");
    }

    @Test()
    public void setExisting() {
        getAlign(2, 3).set(1, 1, "new value");
    }

    @Test()
    public void setExistingCornerCases() {
        getAlign(3, 3).set(0, 1, "new value");
        getAlign(3, 3).set(1, 0, "new value");
        getAlign(3, 3).set(1, 2, "new value");
        getAlign(3, 3).set(2, 1, "new value");
    }

    @Test(expected=IllegalArgumentException.class)
    public void addNegativeColumn() {
        getAlign(2, 3).addColumn(-1, "new");
    }

    @Test(expected=IllegalArgumentException.class)
    public void addNegativeLine() {
        getAlign(2, 3).addLine(-1, "new");
    }

    @Test()
    public void addShortColumn() {
        getAlign(2, 3).addColumn(1, "new");
    }

    @Test()
    public void addShortColumnOutsideField() {
        getAlign(2, 3).addColumn(5, "new");
    }

    @Test()
    public void addLongColumn() {
        getAlign(2, 3).addColumn(1, "new", "new", "new", "new", "new");
    }

    @Test()
    public void addLongColumnOutsideField() {
        getAlign(2, 3).addColumn(5, "new", "new", "new", "new", "new");
    }

    @Test()
    public void addShortLine() {
        getAlign(2, 3).addLine(1, "new");
    }

    @Test()
    public void addShortLineOutsideField() {
        getAlign(2, 3).addLine(5, "new");
    }

    @Test()
    public void addLongLine() {
        getAlign(2, 3).addLine(1, "new", "new", "new", "new", "new");
    }

    @Test()
    public void addLongLineOutsideField() {
        getAlign(2, 3).addLine(5, "new", "new", "new", "new", "new");
    }

    /** TEST ALIGN **/

    @Test()
    public void outputSetExisting() {
        Align align = getAlign(2, 3);
        align.set(1, 1, "new value");
        align.output((String s) -> nothing(s));
    }

    @Test()
    public void outputAddShortColumn() {
        Align align = getAlign(2, 3);
        align.addColumn(1, "new");
        align.output((String s) -> nothing(s));
    }

    @Test()
    public void outputAddShortColumnOutsideField() {
        Align align = getAlign(2, 3);
        align.addColumn(5, "new");
        align.output((String s) -> nothing(s));
    }

    @Test()
    public void outputAddLongColumn() {
        Align align = getAlign(2, 3);
        align.addColumn(1, "new", "new", "new", "new", "new");
        align.output((String s) -> nothing(s));
    }

    @Test()
    public void outputAddLongColumnOutsideField() {
        Align align = getAlign(2, 3);
        align.addColumn(5, "new", "new", "new", "new", "new");
        align.output((String s) -> nothing(s));
    }

    @Test()
    public void outputAddShortLine() {
        Align align = getAlign(2, 3);
        align.addLine(1, "new");
        align.output((String s) -> nothing(s));
    }

    @Test()
    public void outputAddShortLineOutsideField() {
        Align align = getAlign(2, 3);
        align.addLine(5, "new");
        align.output((String s) -> nothing(s));
    }

    @Test()
    public void outputAddLongLine() {
        Align align = getAlign(2, 3);
        align.addLine(1, "new", "new", "new", "new", "new");
        align.output((String s) -> nothing(s));
    }

    @Test()
    public void outputAddLongLineOutsideField() {
        Align align = getAlign(2, 3);
        align.addLine(5, "new", "new", "new", "new", "new");
        align.output((String s) -> nothing(s));
    }

    public void nothing(String s) {
        //System.out.println(s + "\n\n");
    }
}
\$\endgroup\$
10
\$\begingroup\$

You can simplify this:

align.output((String s) -> nothing(s));

to the shorter form:

align.output(this::nothing);

But, the important thing is, as you guessed, that printing from unit tests is not the right thing to do. You should convert the printing to assertions, for example:

@Test()
public void outputAddShortLineOutsideField() {
    Align align = getAlign(2, 3);
    align.addLine(5, "new");
    assertEquals(
            "00     01     \n" +
            "10     11     \n" +
            "20     21     \n" +
            "-      -      \n" +
            "-      -      \n" +
            "new    -      \n", align.toString());
}

@Test()
public void outputAddLongColumn() {
    Align align = getAlign(2, 3);
    align.addColumn(1, "new", "new", "new", "new", "new");
    assertEquals(
            "00     new     01     \n" +
            "10     new     11     \n" +
            "20     new     21     \n" +
            "-      new     -      \n" +
            "-      new     -      \n", align.toString());
}

I recommend to replace all the align.output(this::nothing); lines with this:

assertEquals("", align.toString());

Run it (lots of failures), your IDE should print (somewhere) an error message with the expected and actual values. If the actual values look what you expected, copy-paste into the unit test. Repeat for all tests and you're done. (... or go ahead and refactor some more ;-)

Btw, I think it's not a good idea to use toString for formatting purposes. It would be better to move the formatting logic to a different method (.format() ?), and make .toString() more "technical".

Also, I think Align is not a great name. Something like TabularTextBuilder or just TableBuilder would be better. And it might make sense to borrow ideas from the Builder pattern, making addLine return this instead of void, so that you can chain all the .addLine(...) (and other) calls, ending in a .format() call that returns the formatted text.

\$\endgroup\$
5
\$\begingroup\$

The concept here is great, I like it, and I can see real usefulness for it.

The Content class is unnecessary, it is just a logic container that does not add value. All the methods in Align simply delegate to Content anyway, and there is exactly one content per Align. Instead of Content, you could simply have List<Line> and make the logic in there more private.... I am not sure why Content was protected anyway.

There may be a slight performance benefit to tracking the max-sizes of each column as the data is added, and conditionally re-calculate it if data is removed, or set (and the removed value is the same size as the max for that column).

You can probably save a fair amount of memory churn if you were to pre-calculate the size of the StringBuilder in the toString():

int rowwidth = maxContentLength.stream().sum();
rowwidth += (maxContentLength.size() - 1) * 1 + 1; // spaces between columns and a newline
int charcount = rowwidth * lines.size();
StringBuilder out = new StringBuilder(charcount);
StringBuilder out = new StringBuilder();
\$\endgroup\$
  • \$\begingroup\$ thanks for your answer :) I made the Content class so that I don't have the code that stores stuff in the same class as the code that formats stuff. I also wasn't too happy with it, but I think it's better than having both functionalities in one class. And except for shift, I don't think there are methods which would fit in the Line class. \$\endgroup\$ – tim Oct 2 '14 at 9:46
3
\$\begingroup\$

Changes:

  • more consistent with use of Line instead of List<String> in Table (formerly named Content)
  • extracted some more functionality to functions
  • changed how addColumn works (I think it's a bit clearer to first shift all lines to make space for the column and then setting the values)

From the other answers:

  • pre-calculate StringBuilder size
  • move line-related code to Line
  • protected -> private
  • don't use toString to format
  • updated tests
  • new name for Align and Content
  • return this instead of void for add methods
  • objects can be added directly using a formater

FormatedTableBuilder

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

public class FormatedTableBuilder {

    private String paddingChar = "-";
    private int minSpacing = 5;

    private Table table;

    public FormatedTableBuilder() {
        table = new Table();
    }

    /**
     * set the character that is used for values that don't exist.
     *
     * @param paddingChar padding char
     */
    public void setPaddingChar(String paddingChar) {
        this.paddingChar = paddingChar;
    }

    /**
     * the minimum amount of spaces between columns in the formated output.
     *
     * Must be at least 0, but a minimum of at least 1 is suggested.
     *
     * @param minSpacing minimum of spaces (must be at least 0)
     */
    public void setMinSpacing(int minSpacing) {
        if (minSpacing < 0) {
            throw new IllegalArgumentException("min spacing must be at least 0");
        }
        this.minSpacing = minSpacing;
    }

    public interface Formater<T> {
        String[] format(T object);
    }

    public <T> FormatedTableBuilder add(List<T> objects, Formater<T> formater) {
        objects.forEach((T object) -> addLine(formater.format(object)));
        return this;
    }

    public FormatedTableBuilder addLine(int position, List<String> lineContent) {
        table.addLine(position, new Line(lineContent));
        return this;
    }

    public FormatedTableBuilder addLine(List<String> lineContent) {
        return addLine(table.lineCount(), lineContent);
    }

    public FormatedTableBuilder addLine(String... lineContent) {
        return addLine(table.lineCount(), new ArrayList<>(Arrays.asList(lineContent)));
    }

    public FormatedTableBuilder addLine(int position, String... lineContent) {
        return addLine(position, new ArrayList<>(Arrays.asList(lineContent)));
    }

    public FormatedTableBuilder addColumn(int position, List<String> columnContent) {
        table.addColumn(position, columnContent);
        return this;
    }

    public FormatedTableBuilder addColumn(List<String> columnContent) {
        return addColumn(table.columnCount(), columnContent);
    }

    public FormatedTableBuilder addColumn(String... columnContent) {
        return addColumn(table.columnCount(), new ArrayList<>(Arrays.asList(columnContent)));
    }

    public FormatedTableBuilder addColumn(int position, String... columnContent) {
        return addColumn(position, new ArrayList<>(Arrays.asList(columnContent)));
    }

    /**
     * sets the value at the given column and row to the given content.
     *
     * If the value doesn't exist already, an IllegalArgumentException will be
     * thrown.
     *
     * @param column column
     * @param row row
     * @param value value
     * @return this builder
     */
    public FormatedTableBuilder set(int column, int row, String value) {
        table.set(column, row, value);
        return this;
    }

    /**
     * passes the formated string to consumer.
     *
     * To get the formated string, use format.
     *
     * @param consumer consumer
     */
    public void output(Consumer<String> consumer) {
        consumer.accept(format());
    }

    /**
     * formats all given content so it presents as a well aligned table.
     *
     * @return formated content
     */
    public String format() {
        List<Integer> maxContentLength = getMaxLengths();
        List<String> spaces = getSpaces(maxContentLength);

        // calculate stringbuilder size
        int rowwidth = maxContentLength.stream().mapToInt(i -> i).sum();
        rowwidth += (table.columnCount() - 1) * minSpacing + 1; // spaces between columns and a newline
        int charCount = rowwidth * table.lineCount();

        StringBuilder out = new StringBuilder(charCount);
        for (Line line : table.lines) {
            for (int columnIndex = 0; columnIndex < line.size(); columnIndex++) {
                String spacesForCurrentColumn = spaces.get(columnIndex);
                String column = line.get(columnIndex);
                out.append(column);
                out.append(spacesForCurrentColumn
                        .substring(0,
                                spacesForCurrentColumn.length()
                                - column.length())); // align
            }
            out.append("\n");
        }
        return out.toString();
    }

    @Override
    public String toString() {
        StringBuilder out = new StringBuilder();
        table.lines.forEach(line -> {
            line.forEach(
                    field -> out.append(field).append("            "));
            out.append("\n");
        });
        return out.toString();
    }

    /**
     * returns a list of spaces of the given lengths.
     *
     * @param lengths lengths
     * @return list of spaces
     */
    private List<String> getSpaces(List<Integer> lengths) {
        List<String> spaces = new ArrayList<>();
        lengths.forEach(length -> spaces.add(getSpaces(length)));
        return spaces;
    }

    /**
     * returns n spaces.
     *
     * @param n number of spaces
     * @return n spaces
     */
    private static String getSpaces(int n) {
        StringBuilder spaces = new StringBuilder(n);
        for (int i = 0; i < n; i++) {
            spaces.append(" ");
        }
        return spaces.toString();
    }

    /**
     * returns a list containing the length of the longest string of each
     * column.
     *
     * @return lengths
     */
    private List<Integer> getMaxLengths() {
        List<Integer> maxContentLength = new ArrayList<>();
        for (int i = 0; i < table.columnCount(); i++) {
            maxContentLength.add(0); // init max length with 0
        }

        for (Line line : table.lines) {
            for (int columnIndex = 0; columnIndex < table.columnCount(); columnIndex++) {
                String column = line.get(columnIndex);
                if (column.length() > maxContentLength.get(columnIndex)) {
                    maxContentLength.set(columnIndex, column.length() + minSpacing);
                }
            }
        }
        return maxContentLength;
    }

    private class Table {

        private List<Line> lines;

        public Table() {
            lines = new ArrayList<>();
        }

        public int lineCount() {
            return lines.size();
        }

        public int columnCount() {
            if (lines.isEmpty()) {
                return 0;
            }
            return lines.get(0).size(); // all lines are always the same length because of padding
        }

        public void set(int column, int row, String value) {
            if (row < 0 || row > lineCount()) {
                throw new IllegalArgumentException("row doesn't exist");
            }
            Line line = lines.get(row);
            if (column < 0 || column > columnCount()) {
                throw new IllegalArgumentException("column doesn't exist");
            }
            line.set(column, value);
        }

        public void addColumn(int position, List<String> columnContent) {
            if (position < 0) {
                throw new IllegalArgumentException("Column position is negative");
            }

            // if the new position is outside the current content, extend short lines to position
            if (position > columnCount()) {
                padLinesTo(position);
            }

            // add new lines for content of this column in case the column is too long
            while (lines.size() < columnContent.size()) {
                addLinePaddedTo(position);
            }

            shiftAllAt(position, lines); // make space for new column
            for (int lineIndex = 0; lineIndex < columnContent.size(); lineIndex++) { // add new column
                set(position, lineIndex, columnContent.get(lineIndex));
            }

            if (columnContent.size() > columnCount()) {
                padLinesTo(table.columnCount());
            }
        }

        public void addLine(int position, List<String> columnContent) {
            Line newLine = new Line(columnContent);
            if (position < 0) {
                throw new IllegalArgumentException("Line position is negative");
            }

            // add elements to this line if it is too short
            newLine.padTo(columnCount());

            if (newLine.size() > columnCount()) { // add elements to all other lines if bew line is the new longest line
                padLinesTo(newLine.size()); // padd to new longest line
            }

            while (lines.size() < position) { // add new padded empty lines if new line is outside field
                addLinePaddedTo(columnCount());
            }
            lines.add(position, newLine); // add actual line
        }

        /**
         * adds a new line to the end of the content, padded to the given
         * position.
         *
         * @param padTo padTo
         */
        private void addLinePaddedTo(int padTo) {
            Line emptyLine = new Line();
            emptyLine.padTo(padTo);
            lines.add(emptyLine);
        }

        /**
         * adds padding char to all lines until they are the given length.
         *
         * @param padTo padTo
         */
        private void padLinesTo(int padTo) {
            lines.forEach(line -> line.padTo(padTo));
        }

        /**
         * shifts all given lines at given position one entry to the right.
         *
         * The padding character will be inserted at the new position.
         *
         * @param position position
         * @param list list
         */
        private void shiftAllAt(int position, List<Line> list) {
            list.forEach(line -> line.shiftAt(position));
        }
    }

    private class Line extends ArrayList<String> {

        public Line() {
            super();
        }

        public Line(List<String> line) {
            super(line);
        }

        /**
         * add padding char to this line until its length is the given max.
         *
         * @param max max
         */
        private void padTo(int max) {
            while (this.size() < max) {
                this.add(paddingChar);
            }
        }

        /**
         * inserts the padding char at the given position, thus shifting the
         * rest of the line one position to the right.
         *
         * @param position position
         */
        private void shiftAt(int position) {
            this.add(position, paddingChar);
        }
    }
}

Tests

import static org.junit.Assert.assertEquals;
import org.junit.Test;

public class FormatedTableBuilderTest {

    private static FormatedTableBuilder getAlign(int columns, int rows) {
        FormatedTableBuilder align = new FormatedTableBuilder();
        for (int i = 0; i < rows; i++) {
            String[] row = new String[columns];
            for (int j = 0; j < columns; j++) {
                row[j] = String.valueOf(i) + String.valueOf(j);
            }
            align.addLine(row);
        }
        align.setMinSpacing(5);
        align.setPaddingChar("-");
        return align;
    }

    /**
     * TEST CONTENT *
     */
    @Test(expected = IllegalArgumentException.class)
    public void setNegativeColumn() {
        getAlign(2, 3).set(-1, 1, "new value");
    }

    @Test(expected = IllegalArgumentException.class)
    public void setNonExistingColumn() {
        getAlign(2, 3).set(20, 1, "new value");
    }

    @Test(expected = IllegalArgumentException.class)
    public void setNegativeLine() {
        getAlign(2, 3).set(1, -1, "new value");
    }

    @Test(expected = IllegalArgumentException.class)
    public void setNonExistingLine() {
        getAlign(2, 3).set(1, 20, "new value");
    }

    @Test()
    public void setExisting() {
        getAlign(2, 3).set(1, 1, "new value");
    }

    @Test()
    public void setExistingCornerCases() {
        getAlign(3, 3).set(0, 1, "new value");
        getAlign(3, 3).set(1, 0, "new value");
        getAlign(3, 3).set(1, 2, "new value");
        getAlign(3, 3).set(2, 1, "new value");
    }

    @Test(expected = IllegalArgumentException.class)
    public void addNegativeColumn() {
        getAlign(2, 3).addColumn(-1, "new");
    }

    @Test(expected = IllegalArgumentException.class)
    public void addNegativeLine() {
        getAlign(2, 3).addLine(-1, "new");
    }

    @Test()
    public void addShortColumn() {
        getAlign(2, 3).addColumn(1, "new");
    }

    @Test()
    public void addShortColumnOutsideField() {
        getAlign(2, 3).addColumn(5, "new");
    }

    @Test()
    public void addLongColumn() {
        getAlign(2, 3).addColumn(1, "new", "new", "new", "new", "new");
    }

    @Test()
    public void addLongColumnOutsideField() {
        getAlign(2, 3).addColumn(5, "new", "new", "new", "new", "new");
    }

    @Test()
    public void addShortLine() {
        getAlign(2, 3).addLine(1, "new");
    }

    @Test()
    public void addShortLineOutsideField() {
        getAlign(2, 3).addLine(5, "new");
    }

    @Test()
    public void addLongLine() {
        getAlign(2, 3).addLine(1, "new", "new", "new", "new", "new");
    }

    @Test()
    public void addLongLineOutsideField() {
        getAlign(2, 3).addLine(5, "new", "new", "new", "new", "new");
    }

    /**
     * TEST ALIGN *
     */
    @Test()
    public void outputSetExisting() {
        FormatedTableBuilder align = getAlign(2, 3);
        align.set(1, 1, "new value");
        assertEquals(
                "00     01            \n"
                + "10     new value     \n"
                + "20     21            \n", align.format());
    }

    @Test()
    public void outputAddShortColumn() {
        FormatedTableBuilder align = getAlign(2, 3);
        align.addColumn(1, "new");
        assertEquals(
                "00     new     01     \n"
                + "10     -       11     \n"
                + "20     -       21     \n", align.format());
    }

    @Test()
    public void outputAddShortColumnOutsideField() {
        FormatedTableBuilder align = getAlign(2, 3);
        align.addColumn(5, "new");
        assertEquals(
                "00     01     -     -     -     new     \n"
                + "10     11     -     -     -     -       \n"
                + "20     21     -     -     -     -       \n", align.format());
    }

    @Test()
    public void outputAddLongColumn() {
        FormatedTableBuilder align = getAlign(2, 3);
        align.addColumn(1, "new", "new", "new", "new", "new");
        assertEquals(
                "00     new     01     \n"
                + "10     new     11     \n"
                + "20     new     21     \n"
                + "-      new     -      \n"
                + "-      new     -      \n", align.format());
    }

    @Test()
    public void outputAddLongColumnOutsideField() {
        FormatedTableBuilder align = getAlign(2, 3);
        align.addColumn(5, "new", "new", "new", "new", "new");
        assertEquals(
                "00     01     -     -     -     new     \n"
                + "10     11     -     -     -     new     \n"
                + "20     21     -     -     -     new     \n"
                + "-      -      -     -     -     new     \n"
                + "-      -      -     -     -     new     \n", align.format());
    }

    @Test()
    public void outputAddShortColumnEndOfField() {
        FormatedTableBuilder align = getAlign(2, 3);
        align.addColumn("new");
        assertEquals(
                "00     01     new     \n"
                + "10     11     -       \n"
                + "20     21     -       \n", align.format());
    }

    @Test()
    public void outputAddLongColumnEndOfField() {
        FormatedTableBuilder align = getAlign(2, 3);
        align.addColumn("new", "new", "new", "new", "new");
        assertEquals(
                "00     01     new     \n"
                + "10     11     new     \n"
                + "20     21     new     \n"
                + "-      -      new     \n"
                + "-      -      new     \n", align.format());
    }

    @Test()
    public void outputAddShortLine() {
        FormatedTableBuilder align = getAlign(2, 3);
        align.addLine(1, "new");
        assertEquals(
                "00     01     \n"
                + "new    -      \n"
                + "10     11     \n"
                + "20     21     \n", align.format());
    }

    @Test()
    public void outputAddShortLineOutsideField() {
        FormatedTableBuilder align = getAlign(2, 3);
        align.addLine(5, "new");
        assertEquals(
                "00     01     \n"
                + "10     11     \n"
                + "20     21     \n"
                + "-      -      \n"
                + "-      -      \n"
                + "new    -      \n", align.format());
    }

    @Test()
    public void outputAddLongLine() {
        FormatedTableBuilder align = getAlign(2, 3);
        align.addLine(1, "new", "new", "new", "new", "new");
        assertEquals(
                "00     01     -     -     -     \n"
                + "new    new    new   new   new   \n"
                + "10     11     -     -     -     \n"
                + "20     21     -     -     -     \n", align.format());
    }

    @Test()
    public void outputAddLongLineOutsideField() {
        FormatedTableBuilder align = getAlign(2, 3);
        align.addLine(5, "new", "new", "new", "new", "new");
        assertEquals(
                "00     01     -     -     -     \n"
                + "10     11     -     -     -     \n"
                + "20     21     -     -     -     \n"
                + "-      -      -     -     -     \n"
                + "-      -      -     -     -     \n"
                + "new    new    new   new   new   \n", align.format());
    }
}

Example usage for objects:

    FormatedTableBuilder builder = new FormatedTableBuilder();
    List<User> users = getUsers();

    // format yourself
    for (User user : users) {
        builder.addLine(user.getName(), user.getLastName(), String.valueOf(user.getAge()));
    }
    builder.output((String s) -> System.out.println(s));

    // use formater
    builder.add(users,
            (User user) -> new String[]{user.getName(), user.getLastName(), String.valueOf(user.getAge())});
    builder.output((String s) -> System.out.println(s));
\$\endgroup\$
-3
\$\begingroup\$

This is completely wrong, I mean everything.

  • Not a single useful javadoc or comment
  • Align as a class name? Really?
  • The public interface of the Align class is horrible. Why four addLine and four addColumn methods?
  • Content as a name of the variable is really descriptive.

Here's the idea where to go:

Formatter<User> userFormatter = new Formatter(format);
for (User user : users)
{
    System.out.println(userFormatter.format(user));
}
\$\endgroup\$
  • 3
    \$\begingroup\$ Javadoc or comment are not needed here as his code is readable. Align as a class name looks fine to me, what would you use ? Why four addLine and four addColumn well because there not a unique way to add a line or a column. \$\endgroup\$ – Marc-Andre Oct 2 '14 at 15:02
  • 1
    \$\begingroup\$ yeah, I already changed Align to FormatedTableBuilder and Content to Table. Can you expand on what is wrong with the comments (either in general, or in an example)? There are four add methods for convenience (the caller can add lists or arrays, and define a position to insert or insert at the end without having to pass anything). \$\endgroup\$ – tim Oct 2 '14 at 15:06
  • 1
    \$\begingroup\$ And I'm not sure that your code example would work out very well. First of all, all users are needed to start printing, otherwise it wouldn't be possible to align them properly. And secondly, I'm not sure that it is practical to write it as generic as you are suggesting. But I'll think about that part some more. \$\endgroup\$ – tim Oct 2 '14 at 15:12
  • \$\begingroup\$ The code example works well, format parameter passed to Formatter tells how long the columns should be. How the format parameter is constructed is up to you. \$\endgroup\$ – Epinuj Oct 3 '14 at 7:28
  • 1
    \$\begingroup\$ please elaborate on your bullet points and explain what should be done as opposed to what you say shouldn't be done and explain why. This answer is completely wrong, as far as being a good review. \$\endgroup\$ – Malachi Oct 3 '14 at 14:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.