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;
$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);
java
$endgroup$
migrated from stackoverflow.com Mar 26 at 17:07
This question came from our site for professional and enthusiast programmers.
add a comment |
$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);
java
$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
add a comment |
$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);
java
$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
java
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
add a comment |
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
add a comment |
1 Answer
1
active
oldest
votes
$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$
add a comment |
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
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
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
$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$
add a comment |
$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$
add a comment |
$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$
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.
edited Mar 27 at 0:22
answered Mar 26 at 23:50
potatopotato
42611 bronze badges
42611 bronze badges
add a comment |
add a comment |
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.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
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
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
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
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