Problem
Personally I often prefer console applications over GUI applications.
However, sometimes it is quite a challenge to display everything well-aligned.
Therefore I decided to create a console-based table which makes it possible to display data organized into categories. You can add, remove and update entries and choose between different table designs. This is an example how it looks like.
(Some characters might not be displayed correctly in the Windows console. In the Linux terminal all characters are printed without a problem.)
I’d like to get some feedback what I can improve in this class and if there are any bad habits.
ConsoleTable.h
#ifndef CONSOLETABLE_CONSOLETABLE_H
#define CONSOLETABLE_CONSOLETABLE_H
#include <string>
#include <vector>
#include <iostream>
#include "ConsoleTableRow.h"
#include "ConsoleTableUtils.h"
#include <sstream>
enum TableStyle {
BASIC,
LINED,
DOUBLE_LINE,
};
enum HorizontalSeperator{
SEPERATOR_TOP,
SEPERATOR_MIDDLE,
SEPERATOR_BOTTOM
};
class ConsoleTable {
public:
ConsoleTable(TableStyle style);
void setPadding(unsigned int width);
void addColumn(std::string name);
void addRow(ConsoleTableRow *item);
bool removeRow(int index);
bool editRow(std::string data, int row, int col);
void printTable();
private:
unsigned int padding = 1;
std::vector<std::string> columns;
std::vector<ConsoleTableRow *> entries;
ConsoleTableUtils* utils;
// Table Style variables
std::string style_line_horizontal;
std::string style_line_vertical;
std::string style_line_cross;
std::string style_t_intersect_right;
std::string style_t_intersect_left;
std::string style_t_intersect_top;
std::string style_t_intersect_bottom;
std::string style_edge_topleft;
std::string style_edge_topright;
std::string style_edge_buttomleft;
std::string style_edge_buttomright;
void printHorizontalSeperator(const std::vector<int> &maxWidths, HorizontalSeperator seperator) const;
void setTableStyle(TableStyle style);
};
#endif //CONSOLETABLE_CONSOLETABLE_H
ConsoleTable.cpp
#include "ConsoleTable.h"
ConsoleTable::ConsoleTable(TableStyle style) {
setTableStyle(style);
this->utils = new ConsoleTableUtils();
}
void ConsoleTable::addColumn(std::string name) {
this->columns.push_back(name);
}
void ConsoleTable::printTable() {
// Calculate column maxima
std::vector<int> maxWidths(this->columns.size());
for (int row = 0; row < this->entries.size(); row++) {
for (int col = 0; col < this->columns.size(); col++) {
std::string cellText = this->entries[row]->getEntry()[col];
if (this->columns[col].length() > maxWidths[col])
maxWidths[col] = this->columns[col].length();
if (maxWidths[col] < cellText.length()) {
maxWidths[col] = cellText.length();
}
}
}
printHorizontalSeperator(maxWidths, SEPERATOR_TOP);
// Print column values
for (int col = 0; col < this->columns.size(); col++) {
std::string cellText = this->columns[col];
int len = cellText.length();
std::string paddedText = cellText + std::string(maxWidths[col] - len, ' ');
std::cout << this->style_line_vertical << std::string(this->padding, ' ') << paddedText
<< std::string(this->padding, ' ');
std::cout << (col == this->columns.size() - 1 ? this->style_line_vertical + "n" : "");
}
printHorizontalSeperator(maxWidths, SEPERATOR_MIDDLE);
// Print cell values
for (int row = 0; row < this->entries.size(); row++) {
for (int col = 0; col < this->columns.size(); col++) {
std::string cellText = this->entries[row]->getEntry()[col];
std::string paddedText = cellText + std::string(maxWidths[col] - cellText.length(), ' ');
std::cout << this->style_line_vertical << std::string(this->padding, ' ') << paddedText
<< std::string(this->padding, ' ');
}
std::cout << this->style_line_vertical << std::endl;
if (row == this->entries.size() - 1)
printHorizontalSeperator(maxWidths, SEPERATOR_BOTTOM);
else
printHorizontalSeperator(maxWidths, SEPERATOR_MIDDLE);
}
}
void ConsoleTable::printHorizontalSeperator(const std::vector<int> &maxWidths, HorizontalSeperator seperator) const {
for (int col = 0; col < columns.size(); ++col) {
switch (seperator) {
case SEPERATOR_TOP: {
std::cout << (col == 0 ? this->style_edge_topleft : "");
std::cout << utils->repeatString(this->style_line_horizontal, this->padding);
std::cout << utils->repeatString(this->style_line_horizontal, maxWidths[col]);
std::cout << utils->repeatString(this->style_line_horizontal, this->padding);
std::cout << (col != columns.size() - 1 ? this->style_t_intersect_top : this->style_edge_topright);
std::cout << (col == columns.size() - 1 ? "n" : "");
break;
}
case SEPERATOR_MIDDLE: {
std::cout << (col == 0 ? this->style_t_intersect_left : "");
std::cout << utils->repeatString(this->style_line_horizontal, this->padding);
std::cout << utils->repeatString(this->style_line_horizontal, maxWidths[col]);
std::cout << utils->repeatString(this->style_line_horizontal, this->padding);
std::cout << (col != columns.size() - 1 ? this->style_line_cross : this->style_t_intersect_right);
std::cout << (col == columns.size() - 1 ? "n" : "");
break;
}
case SEPERATOR_BOTTOM: {
std::cout << (col == 0 ? this->style_edge_buttomleft : "");
std::cout << utils->repeatString(this->style_line_horizontal, this->padding);
std::cout << utils->repeatString(this->style_line_horizontal, maxWidths[col]);
std::cout << utils->repeatString(this->style_line_horizontal, this->padding);
std::cout
<< (col != columns.size() - 1 ? this->style_t_intersect_bottom : this->style_edge_buttomright);
std::cout << (col == columns.size() - 1 ? "n" : "");
break;
}
}
}
}
void ConsoleTable::addRow(ConsoleTableRow *item) {
this->entries.push_back(item);
}
bool ConsoleTable::removeRow(int index) {
if (index > this->entries.size())
return false;
this->entries.erase(this->entries.begin() + index);
return true;
}
bool ConsoleTable::editRow(std::string data, int row, int col) {
if(row > this->entries.size())
return false;
if(col > this->columns.size())
return false;
auto entry = this->entries[row];
entry->editEntry(data, col);
return true;
}
void ConsoleTable::setPadding(unsigned int width) {
this->padding = width;
}
void ConsoleTable::setTableStyle(TableStyle style) {
switch (style) {
case BASIC: {
this->style_line_horizontal = "-";
this->style_line_vertical = "|";
this->style_line_cross = "+";
this->style_t_intersect_right = "+";
this->style_t_intersect_left = "+";
this->style_t_intersect_top = "+";
this->style_t_intersect_bottom = "+";
this->style_edge_topleft = "+";
this->style_edge_topright = "+";
this->style_edge_buttomleft = "+";
this->style_edge_buttomright = "+";
break;
}
case LINED: {
this->style_line_horizontal = "━";
this->style_line_vertical = "┃";
this->style_line_cross = "╋";
this->style_t_intersect_right = "┫";
this->style_t_intersect_left = "┣";
this->style_t_intersect_top = "┳";
this->style_t_intersect_bottom = "┻";
this->style_edge_topleft = "┏";
this->style_edge_topright = "┓";
this->style_edge_buttomleft = "┗";
this->style_edge_buttomright = "┛";
break;
}
case DOUBLE_LINE: {
this->style_line_horizontal = "═";
this->style_line_vertical = "║";
this->style_line_cross = "╬";
this->style_t_intersect_right = "╣";
this->style_t_intersect_left = "╠";
this->style_t_intersect_top = "╦";
this->style_t_intersect_bottom = "╩";
this->style_edge_topleft = "╔";
this->style_edge_topright = "╗";
this->style_edge_buttomleft = "╚";
this->style_edge_buttomright = "╝";
break;
}
}
}
ConsoleTableRow.h
#ifndef CONSOLETABLE_CONSOLETABLEENTRY_H
#define CONSOLETABLE_CONSOLETABLEENTRY_H
#include <string>
#include <vector>
class ConsoleTableRow {
public:
ConsoleTableRow(int width);
void addEntry(std::string data, int column);
void editEntry(std::string data, int column);
std::vector <std::string> getEntry();
private:
std::vector <std::string> row;
};
#endif //CONSOLETABLE_CONSOLETABLEENTRY_H
ConsoleTableRow.cpp
#include "ConsoleTableRow.h"
ConsoleTableRow::ConsoleTableRow(int width) {
this->row.resize(width);
}
void ConsoleTableRow::addEntry(std::string data, int column) {
row[column] = data;
}
std::vector<std::string> ConsoleTableRow::getEntry() {
return this->row;
}
void ConsoleTableRow::editEntry(std::string data, int column) {
this->row[column] = data;
}
ConsoleTableUtils.h
#ifndef CONSOLETABLE_CONSOLETABLEUTILS_H
#define CONSOLETABLE_CONSOLETABLEUTILS_H
#include <string>
#include <sstream>
class ConsoleTableUtils {
public:
std::string repeatString(std::string input, int n) const;
};
#endif //CONSOLETABLE_CONSOLETABLEUTILS_H
ConsoleTableUtils.cpp
#include "ConsoleTableUtils.h"
std::string ConsoleTableUtils::repeatString(std::string input, int n) const {
std::ostringstream os;
for (int i = 0; i < n; i++)
os << input;
return os.str();
}
Here is an example to generate use the classes.
This example will generate the tables that can be seen in the screenshot above.
#include <iostream>
#include <unistd.h>
#include "ConsoleTable.h"
int main() {
ConsoleTable ct(BASIC);
ct.setPadding(1);
ct.addColumn("Country");
ct.addColumn("Name");
ct.addColumn("Profession");
ct.addColumn("Age");
auto entry = new ConsoleTableRow(4);
entry->addEntry("Germany", 0);
entry->addEntry("Michael", 1);
entry->addEntry("Computer Engineer", 2);
entry->addEntry("19", 3);
ct.addRow(entry);
auto entry2 = new ConsoleTableRow(4);
entry2->addEntry("England", 0);
entry2->addEntry("Robert", 1);
entry2->addEntry("Artist", 2);
entry2->addEntry("34", 3);
ct.addRow(entry2);
auto entry3 = new ConsoleTableRow(4);
entry3->addEntry("United Kingdom", 0);
entry3->addEntry("Julia", 1);
entry3->addEntry("Designer", 2);
entry3->addEntry("42", 3);
ct.addRow(entry3);
auto entry4 = new ConsoleTableRow(4);
entry4->addEntry("United Staates", 0);
entry4->addEntry("Jo", 1);
entry4->addEntry("Actor", 2);
entry4->addEntry("21", 3);
ct.addRow(entry4);
// Print all entries
ct.printTable();
return 0;
}
The project repository can be found on GitHub.
Solution
I see a number of things that may help you improve your code.
Think of the user
This is rarely the first suggestion I make, but in this case, it seems particularly important. The problem is that the addition of rows seems to require that the user keep track of the column number and the syntax is quite verbose. Instead of this:
auto entry2 = new ConsoleTableRow(4);
entry2->addEntry("England", 0);
entry2->addEntry("Robert", 1);
entry2->addEntry("Artist", 2);
entry2->addEntry("34", 3);
ct.addRow(entry2);
ct.printTable();
I would greatly prefer to write this:
ct += {"England", "Robert", "Artist", "34"};
std::cout << ct;
A few of the suggestions below relate to how one might do this.
Use only necessary #includes
The #include <unistd.h> line in the main program is not necessary and can be safely removed. Also, <iostream> is certainly needed in ConsoleTable.cpp but not in ConsoleTable.h because the interface makes no use of it (even though the implemetation does.)
Think carefully about global enums
The TableStyle and HorizontalSeparator enums only really have relevance within the ConsoleTable class. For that reason, I’d put the TableStyle inside the ConsoleTable class and move the HorizontalSeparator enum to the ConsoleTable.cpp implementation file. Nothing outside that class should ever touch it.
Be consistent with naming
Speaking of TableStyle, it’s strange to have LINED (with the D at the end) but DOUBLE_LINE (without the D at the end). It’s a minor thing, but this sort of inconsistency can annoy users of the class.
Simplify your code
The printHorizontalSeperator code is quite long, repetitive and difficult to read. Also, there are only ever three types of lines: line, middle and bottom. I’d suggest instead to simplify this greatly. First, let’s make it much easier to add styles. I’d add this to the ConsoleTable class:
static constexpr std::string_view markers[3][11] {
{ "-","|",
"+","+","+",
"+","+","+",
"+","+","+"},
{ "━","┃",
"┏","┳","┓",
"┣","╋","┫",
"┗","┻","┛"},
{ "═","║",
"╔","╦","╗",
"╠","╬","╣",
"╚","╩","╝"},
};
Two things to note here. First, I’m using C++17 and so std::string_view, allowing for this to be constexpr, but if you don’t have that, it’s simple enough to make them plain const std::string instead. Second, the way the characters are physically arranged makes it much simpler to visually verify that the characters are correct.
Next, I’d recommend creating a private member function like this:
std::string line(unsigned n) const;
This constructs the top, middle or bottom line and returns a single string. Here’s how I wrote it:
std::string ConsoleTable::line(unsigned n) const {
std::stringstream line;
n *= 3;
line << markers[linetype][2+n];
for (std::size_t i{0}; i < widths.size()-1; ++i) {
for (std::size_t j{0}; j < (widths[i] + padsize + padsize); ++j) {
line << markers[linetype][0];
}
line << markers[linetype][3+n];
}
for (std::size_t j{0}; j < (widths.back() + padsize + padsize); ++j) {
line << markers[linetype][0];
}
line << markers[linetype][4+n] << 'n';
return line.str();
}
Here is how the private member data variables are declared:
std::size_t padsize;
Style linetype;
bool innerlines;
std::vector<std::string> header;
std::vector<std::size_t> widths;
std::vector<std::vector<std::string>> rows;
As you can probably infer, I’ve renamed TableStyle to Style and put it within the class definition.
Use std::initializer_list to simplify code
Reading the code, it seemed that the most fundamental part of the ConsoleTable was not the line style (which the current constructor uses) but instead, the names of the columns. Here’s the constructor that uses std::initializer_list to greatly simplify the code:
ConsoleTable::ConsoleTable(std::initializer_list<std::string> list) :
padsize{1},
linetype{BASIC},
innerlines{false},
header{list}
{
updateWidths();
}
Here I’m using the C++11 unified constructor syntax (with the {}) so it’s unambiguous that these are not function calls
Overload operators to make the syntax clean
The use of the += operator in my proposed example above is not at all difficult to write:
ConsoleTable &ConsoleTable::operator+=(std::initializer_list<std::string> row) {
if (row.size() > widths.size()) {
throw std::invalid_argument{"appended row size must be same as header size"};
}
std::vector<std::string> r = std::vector<std::string>{row};
rows.push_back(r);
for (std::size_t i{0}; i < r.size(); ++i) {
widths[i] = std::max(r[i].size(), widths[i]);
}
return *this;
}
This code doesn’t use explicit dynamic memory allocation, but it certainly could with minor changes. The nice thing is that if you want to change that, it’s a detail that’s hidden within the class implementation and the interface could remain the same.
Use const where possible
The printTable() function does not (and should not) alter the underlying data structure and should therefore be declared const. Using const wherever it’s practical to do so is a very easy way to get good performance from your code.
Use an ostream &operator<< instead of printTable
The current code has void ConsoleTable::printTable() but what would make more sense and be more general purpose would be to overload an ostream operator<< instead. This allows the user of the code to direct the output to the std::cout or any other convenient ostream. The declaration looks like this:
friend std::ostream &operator<<(std::ostream &out, const ConsoleTable &t);
The implementation looks like this:
std::ostream &operator<<(std::ostream &out, const ConsoleTable &t)
{
out << t.line(0);
t.printRow(out, t.header);
auto mid = t.line(1);
if (!t.innerlines) {
out << mid;
mid.erase();
}
for (const auto &row : t.rows) {
out << mid;
t.printRow(out, row);
}
return out << t.line(2);
}
Note that this uses a private member function printRow() to print each std::vector<std::string> for either the header or the data. It also uses the previously shown private member line function to create the upper, middle and lower lines (each just once) to print. Also, I’ve added a feature which can be turned on or off via the boolean variable innerlines. When set to true, it prints lines between each data row, but when set to false, it omits those inner lines and just prints each data row with no visual separator. I find this looks cleaner and also allows more data to be shown on the screen at once.
Consolidate classes
There doesn’t really seem to be a strong need for a ConsoleTableRow class here. I’d suggest instead to keep the rows internally like this:
std::vector<std::vector<std::string>> rows;
Since this is just an implementation detail once we use std::initializer_lists as mentioned above (that is, the internal representation of the rows are no longer part of the interface), it could always be added if really needed.
Use namespaces where appropriate
The ConsoleTableUtils object shouldn’t really be an object at all. It should be a namespace or simply make repeatString a freestanding function if it’s needed at all. I’d say it’s not really needed since it could just as easily be done with one of the If needed, it could be written like this:std::string constructors directly.
std::string operator*(const std::string &other, std::size_t repeats) {
std::string ret;
ret.reserve(other.size() * repeats);
for ( ; repeats; --repeats)
ret.append(other);
return ret;
}
This gives us a handy and intuitive syntax:
std::string foo{"foo"};
std::cout << "String testn" << (foo * 3) << 'n';
Don’t write this->
In member functions, adding this-> everywhere only clutters up the code and makes it harder to read because this-> is implicit.
Combine parameter setting for convenience
There is a function called setTableStyle which takes a single parameter which is actually just the type of line to be used. I’d argue that the “style” might include the padding as well, and in my version, whether to print inner lines or not. Finally, the name setTableStyle is somewhat redundant since it’s a member function of a table. I’s shorten that to style since Table is implicit because it’s a table object, and set is implicit when you pass a value. Here’s how I’d write it:
void ConsoleTable::style(ConsoleTable::Style s, std::size_t padsize, bool innerlines) {
linetype = s;
padsize = padsize;
innerlines = innerlines;
}
I suggest you to use Fluent API for ConsoleTable class.
You can use unique_ptr instead of naked pointer, then did not worry about memory leak.
I think repeatString function does not need a class, and write like alone function in namespace like namespace console_table, if you like a class, please declare repeatString as static function to prevent extra instantiation.
Please use use keyword or typedef to rename a better name for std::vector<std::string> to something like Rows. please see When should I use typedef in C++?
I think getEntry() must be right const, and return const reference.
Use enum class instead of enum.
Use const whenever possible, right const functions follow this rule too.
I prefer decouple cout from ConsoleTable and use ostream instead of. because we able to draw table in any place I want. for example in file.
Use const reference input for std::string in this situation.
In ConsoleTableRow correct guard name, you write #ifndef CONSOLETABLE_CONSOLETABLEENTRY_H instead of #ifndef CONSOLETABLE_CONSOLETABLEROW_H
I suggest you to change TableStyle to struct and injected to ConsoleTable instead of use some limit type, and create three style like above for default instantiation of this struct. in this situation, your library has been more flexibility. and users of your library can create their favorite style.
Don’t really have time to go over everything in detail, but here’s some things that I noticed which haven’t been mentioned by sorosh_sabz yet.
- No need to use
this->in your member functions, it’s implicit. See When should I make explicit use of thethispointer? for when it’s actually required. - Prefer using
.at()instead ofoperator[]when accessing vectors. E.g. in yourConsoleTableRowclass functions. Since you don’t have direct control over the index (the user supplies it), the given index might be out of range. The.at()function will do bounds checking for you, whereasoperator[]won’t. So usingoperator[]is only recommended when you can be 100% certain there won’t be an out of bounds access. - As sorosh_sabs mentioned, you should use
std::unique_ptrinstead of raw pointers, because you’ve got memory leaks all over the place currently. However, I don’t see why you actually need pointers add all. Why not simply pass aConsoleTableRowinstead of anstd::unique_ptr<ConsoleTableRow>toConsoleTable.addRow()? - Needs more DRY. E.g. the function
ConsoleTable.printHorizontalSeperatorhas a lot of code duplication.
-
What is the
ConsoleTableUtilsclass for? It has no members, apart from a method that probably ought to be a function instead. -
Instead of the
TableStyleenum, consider making a style type, containing the characters to be used (and provide some standard styles). We can then decouple the contents from the formatting (e.g. we might want to use different style for printing and for on-screen display). -
Why can I write only to
std::cout? Don’t make it hard to write to other streams. -
There’s no checking that
addRowis given a row of the right length. And there’s nostd::initializer_listconstructor to create a row as a one-liner. -
If we output the “middle separator” at the start of each row, we don’t need to special-case the last row:
printHorizontalSeparator(maxWidths, SEPARATOR_TOP); printRow(headers); for (row: rows) { printHorizontalSeparator(maxWidths, SEPARATOR_MIDDLE); printRow(row); } printHorizontalSeparator(maxWidths, SEPARATOR_BOTTOM); -
Reduce duplication in
printHorizontalSeparator: decide which four characters you’ll be using (start, middle, cross and end), and after that all three code blocks collapse into one. -
Consider supporting other character types (specifically,
std::wstringis very useful). -
Add support for per-column alignment (left, right, centre, or numeric).
-
Spelling – “Separator“