Improving my Java class that prints tabular outputImproving the Java UUID class performanceAvoid duplicate conditional checks in multiple boolean conditionsA Java class that prints a matrixGolf some Code with A-RayImplementation of stackCompare 2 unordered, rooted trees for shape-isomorphismFollow-up 1: Compare 2 unordered, rooted trees for shape-isomorphismComparing each element of a list to all other elements of the same listImproving Java LinkList implementationOptimizing Recursion in Knight's Tour

How to interpret a promising preprint that was never published in peer-review?

Why are there few or no black super GMs?

How to tell if JDK is available from within running JVM?

Did Hitler say this quote about homeschooling?

The most secure way to handle someone forgetting to verify their account?

What is the name for the average of the largest and the smallest values in a given data set?

Can error correction and detection be done without adding extra bits?

How many opportunity attacks can you make per turn before becoming exhausted?

Why do jet engines sound louder on the ground than inside the aircraft?

Why would word of Princess Leia's capture generate sympathy for the Rebellion in the Senate?

Inscriptio Labyrinthica

When designing an adventure, how can I ensure a continuous player experience in a setting that's likely to favor TPKs?

Improving an O(N^2) function (all entities iterating over all other entities)

Can't install anything via terminal

Why did my "seldom" get corrected?

Which GPUs to get for Mathematical Optimization (if any)?

How was Luke's prosthetic hand in Episode V filmed?

Transistor power dissipation rating

Who determines when road center lines are solid or dashed?

Is it legal for a supermarket to refuse to sell an adult beer if an adult with them doesn’t have their ID?

Authorship dispute on a paper that came out of a final report of a course?

Why teach C using scanf without talking about command line arguments?

Align the contents of a numerical matrix when you have minus signs

Who or what determines if a curse is valid or not?



Improving my Java class that prints tabular output


Improving the Java UUID class performanceAvoid duplicate conditional checks in multiple boolean conditionsA Java class that prints a matrixGolf some Code with A-RayImplementation of stackCompare 2 unordered, rooted trees for shape-isomorphismFollow-up 1: Compare 2 unordered, rooted trees for shape-isomorphismComparing each element of a list to all other elements of the same listImproving Java LinkList implementationOptimizing Recursion in Knight's Tour






.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;








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 contentnYou wish to print must be the same as amount of items in headers withnthe exception of situation where you want program to add index. In thisncase headers should contain 1 more item than each sublist of content to print");
System.exit(0);












share|improve this question









$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

















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 contentnYou wish to print must be the same as amount of items in headers withnthe exception of situation where you want program to add index. In thisncase headers should contain 1 more item than each sublist of content to print");
System.exit(0);












share|improve this question









$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













1












1








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 contentnYou wish to print must be the same as amount of items in headers withnthe exception of situation where you want program to add index. In thisncase headers should contain 1 more item than each sublist of content to print");
System.exit(0);












share|improve this question









$endgroup$




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 contentnYou wish to print must be the same as amount of items in headers withnthe exception of situation where you want program to add index. In thisncase headers should contain 1 more item than each sublist of content to print");
System.exit(0);









java






share|improve this question













share|improve this question











share|improve this question




share|improve this question










asked Mar 26 at 10:20







Michał Biel











migrated from stackoverflow.com Mar 26 at 17:07


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









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












  • 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







2




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




$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










1 Answer
1






active

oldest

votes


















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.






share|improve this answer











$endgroup$















    Your Answer






    StackExchange.ifUsing("editor", function ()
    StackExchange.using("externalEditor", function ()
    StackExchange.using("snippets", function ()
    StackExchange.snippets.init();
    );
    );
    , "code-snippets");

    StackExchange.ready(function()
    var channelOptions =
    tags: "".split(" "),
    id: "196"
    ;
    initTagRenderer("".split(" "), "".split(" "), channelOptions);

    StackExchange.using("externalEditor", function()
    // Have to fire editor after snippets, if snippets enabled
    if (StackExchange.settings.snippets.snippetsEnabled)
    StackExchange.using("snippets", function()
    createEditor();
    );

    else
    createEditor();

    );

    function createEditor()
    StackExchange.prepareEditor(
    heartbeatType: 'answer',
    autoActivateHeartbeat: false,
    convertImagesToLinks: false,
    noModals: true,
    showLowRepImageUploadWarning: true,
    reputationToPostImages: null,
    bindNavPrevention: true,
    postfix: "",
    imageUploader:
    brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
    contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
    allowUrls: true
    ,
    onDemand: true,
    discardSelector: ".discard-answer"
    ,immediatelyShowMarkdownHelp:true
    );



    );













    draft saved

    draft discarded


















    StackExchange.ready(
    function ()
    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f216274%2fimproving-my-java-class-that-prints-tabular-output%23new-answer', 'question_page');

    );

    Post as a guest















    Required, but never shown
























    1 Answer
    1






    active

    oldest

    votes








    1 Answer
    1






    active

    oldest

    votes









    active

    oldest

    votes






    active

    oldest

    votes









    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.






    share|improve this answer











    $endgroup$

















      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.






      share|improve this answer











      $endgroup$















        3












        3








        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.






        share|improve this answer











        $endgroup$



        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.







        share|improve this answer














        share|improve this answer



        share|improve this answer








        edited Mar 27 at 0:22

























        answered Mar 26 at 23:50









        potatopotato

        42611 bronze badges




        42611 bronze badges



























            draft saved

            draft discarded
















































            Thanks for contributing an answer to Code Review Stack Exchange!


            • Please be sure to answer the question. Provide details and share your research!

            But avoid


            • Asking for help, clarification, or responding to other answers.

            • Making statements based on opinion; back them up with references or personal experience.

            Use MathJax to format equations. MathJax reference.


            To learn more, see our tips on writing great answers.




            draft saved


            draft discarded














            StackExchange.ready(
            function ()
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f216274%2fimproving-my-java-class-that-prints-tabular-output%23new-answer', 'question_page');

            );

            Post as a guest















            Required, but never shown





















































            Required, but never shown














            Required, but never shown












            Required, but never shown







            Required, but never shown

































            Required, but never shown














            Required, but never shown












            Required, but never shown







            Required, but never shown







            Popular posts from this blog

            Kamusi Yaliyomo Aina za kamusi | Muundo wa kamusi | Faida za kamusi | Dhima ya picha katika kamusi | Marejeo | Tazama pia | Viungo vya nje | UrambazajiKuhusu kamusiGo-SwahiliWiki-KamusiKamusi ya Kiswahili na Kiingerezakuihariri na kuongeza habari

            Swift 4 - func physicsWorld not invoked on collision? The Next CEO of Stack OverflowHow to call Objective-C code from Swift#ifdef replacement in the Swift language@selector() in Swift?#pragma mark in Swift?Swift for loop: for index, element in array?dispatch_after - GCD in Swift?Swift Beta performance: sorting arraysSplit a String into an array in Swift?The use of Swift 3 @objc inference in Swift 4 mode is deprecated?How to optimize UITableViewCell, because my UITableView lags

            Access current req object everywhere in Node.js ExpressWhy are global variables considered bad practice? (node.js)Using req & res across functionsHow do I get the path to the current script with Node.js?What is Node.js' Connect, Express and “middleware”?Node.js w/ express error handling in callbackHow to access the GET parameters after “?” in Express?Modify Node.js req object parametersAccess “app” variable inside of ExpressJS/ConnectJS middleware?Node.js Express app - request objectAngular Http Module considered middleware?Session variables in ExpressJSAdd properties to the req object in expressjs with Typescript