1
\$\begingroup\$

I started learning Java not long ago. Quite often, assignments require me to print output in a table. I had some spare time last week, and I wrote a separate class for printing tables. It is working as intended, but I'm not confident the code is "clean". I would like to hear some opinions on what I should improve.

import java.util.List;

import java.util.ArrayList;

/**
 * This class is used to print adjustable table. Table will generate vertical collumns in number
 * corresponding to the number of of objects in headers list which is first parameter taken by constructor.
 * Second parameter is list of lists containing String formated fields user needs printed in a table.
 * The table will adjust fields width to be the length of longest String passed to the constructor.
 * All fields will be centered. Last parameter of constructor of boolean type determines wether
 * user wants to add indexing in first collumn of the table. In such case length of each sublist of 
 * content passed to the constructor should be one field shorter than headers list. Otherwise both
 * lengths should be extacly the same. None of the lists can be empty.
 */

class TablePrinter {

  private List < String > headers;
  private List < List < String >> content;
  private boolean wantIndex;


  public TablePrinter(List < String > headers, List < List < String >> content, boolean wantIndex) {
    this.headers = headers;
    this.content = content;
    this.wantIndex = wantIndex;
  }

  public void printTable() {
    checkIfNoProblems();
    List < String > table = new ArrayList < > ();
    List < List < String >> input = processData();
    List < Integer > collumnsWidth = getCollumnsWidth();
    String fieldSeparator = "│";
    String tableRowSeparator = generateTopMiddleOrBottomLine(collumnsWidth, "rowSeparator");

    table.add(generateTopMiddleOrBottomLine(collumnsWidth, "top"));
    for (List < String > list: input) {
      table.add(fieldSeparator);
      for (int index = 0; index < list.size(); index++) {
        table.add(centerField(list.get(index), collumnsWidth.get(index)));
        table.add(fieldSeparator);
      }
      table.add(tableRowSeparator);
    }
    table.set(table.size() - 1, generateTopMiddleOrBottomLine(collumnsWidth, "bottom"));
    System.out.println(String.join("", table));
  }

  private List < Integer > getCollumnsWidth() {
    List < Integer > collumnsWidth = new ArrayList < > ();

    for (String string: headers) {
      collumnsWidth.add(string.length());
    }
    for (List < String > list: content) {
      for (int index = 0; index < list.size(); index++) {
        if (collumnsWidth.get(index) < list.get(index).length()) {
          collumnsWidth.set(index, list.get(index).length());
        }
      }
    }
    return collumnsWidth;
  }

  private String generateTopMiddleOrBottomLine(List < Integer > collumnsWidth, String modifier) {
    List < String > output = new ArrayList < > ();
    String start;
    String middle;
    String end;

    switch (modifier) {
      case "top":
      default:
        start = "\n╭";
        middle = "┬";
        end = "╮\n";
        break;
      case "rowSeparator":
        start = "\n├";
        middle = "┼";
        end = "┤\n";
        break;
      case "bottom":
        start = "\n╰";
        middle = "┴";
        end = "╯\n";
        break;
    }
    output.add(start);
    for (int number: collumnsWidth) {
      int repetitions = number;
      while (repetitions > 0) {
        output.add("─");
        repetitions--;
      }
      output.add(middle);
    }
    output.set(output.size() - 1, end);
    return String.join("", output);
  }

  private String centerField(String field, int collumnWidth) {
    List < String > centeredField = new ArrayList < > ();
    boolean flip = true;
    int repetitions = collumnWidth - field.length();
    int shift = 1;

    centeredField.add(field);
    while (repetitions > 0) {
      if (flip == true) {
        centeredField.add(centeredField.size() - shift, " ");
      } else {
        centeredField.add(" ");
      }
      flip = !flip;
      shift++;
      repetitions--;
    }
    return String.join("", centeredField);
  }

  private List < List < String >> injectIndex() {
    if (wantIndex) {
      List < List < String >> indexedContent = content;
      Integer index = 1;
      for (List < String > list: indexedContent) {
        list.add(0, index.toString());
        index++;
      }
      return indexedContent;
    }
    return content;
  }

  private List < List < String >> processData() {
    List < List < String >> input = injectIndex();
    input.add(0, headers);
    return input;
  }

  private boolean areListsLengthsValid() {
    int listsLength = headers.size();

    if (wantIndex) {
      listsLength--;
    }
    for (List < String > list: content) {
      if (list.size() != listsLength) {
        return false;
      }
    }
    return true;
  }

  private boolean checkIfListsArentEmpty() {
    List < List < String >> headersAndContent = new ArrayList < > ();

    headersAndContent.add(headers);
    for (List < String > list: content) {
      headersAndContent.add(list);
    }
    for (List < String > list: headersAndContent) {
      try {
        String string = list.get(0);
      } catch (IndexOutOfBoundsException e) {
        return false;
      }
    }
    return true;
  }

  private void checkIfNoProblems() {
    if (checkIfListsArentEmpty() == false) {
      System.out.println("Error, some of lists passed to printer are empty");
      System.exit(0);
    } else if (areListsLengthsValid() == false) {
      System.out.println("Lists lengths are invalid. Amount of items in each sublist of content\nYou wish to print must be the same as amount of items in headers with\nthe exception of situation where you want program to add index. In this\ncase headers should contain 1 more item than each sublist of content to print");
      System.exit(0);
    }
  }
}
\$\endgroup\$

migrated from stackoverflow.com Mar 26 at 17:07

This question came from our site for professional and enthusiast programmers.

  • 2
    \$\begingroup\$ Your prolific use of whitespace in parameterized types (such as "List < List < String >>") is not customary to Java. Use the formatting found in the Java API documentation (e.g. "List<List<String>>") \$\endgroup\$ – TorbenPutkonen Mar 27 at 6:33
3
\$\begingroup\$

Because you asked only about whether this is "clean java" I won't comment on the flaws of this implementation algorithm-wise.


Conventionally methods that check if something is true are named as follows:

boolean isEmpty() // when referring to the object itself
boolean somethingIsEmpty() // when referring to something within the object

It would be more convenient if you renamed methods like checkIfListsArentEmpty() to this format, making it listsAreEmpty() (returning true when they are empty, not the opposite). This would make the code nicer and more readable without double negatives:

if (listsAreEmpty())

As opposed to what you have (checking if the lists are not not empty)


private String generateTopMiddleOrBottomLine(List < Integer > collumnsWidth, String modifier) {...}

You should add documentation for this method to make it clear what values are accepted for modifier. In similar cases with a wider variety of accepted values it'd be even better to use an enum. And again, you use unnecessarily long names.


Using System.exit(0) when this class gets an invalid input is bad design. It might be fine for your current purposes, but the proper way to implement it is to throw an exception that can be handled by a try-catch block instead of forcing the program to shut down immediately.


private List < Integer > getCollumnsWidth() {...}

In such cases it's better to use an array instead of a List of boxed types, since you already know the number of elements to be returned and don't actually need a list of mutable length. It is also better for performance.


You pass around the list of column widths from method to method instead of making it a private field that every method can access. The value can be assigned once during the construction of the object.

\$\endgroup\$

Your Answer

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