12
\$\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\$

    4 Answers 4

    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\$
      1
      • \$\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
        CommentedOct 2, 2014 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\$
        6
        • 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\$CommentedOct 2, 2014 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
          CommentedOct 2, 2014 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
          CommentedOct 2, 2014 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
          CommentedOct 3, 2014 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
          CommentedOct 3, 2014 at 14:27

        Start asking to get answers

        Find the answer to your question by asking.

        Ask question

        Explore related questions

        See similar questions with these tags.