Color ASCII drawing class












7












$begingroup$


I am working on a pseudo graphical interface for a Chess engine I wrote. I want to draw a colored Chess board with ASCII pieces. To abstract the pure std::cout << std::endl; I wrote this little class to organize an ASCII-character "framebuffer":



#include <iostream>
#include <sys/ioctl.h>
#include <unistd.h>
#include <vector>
#include <string>
#include <cassert>
#include <chrono>
#include <thread>

struct Color
{
unsigned char r;
unsigned char g;
unsigned char b;
};

class Framebuffer
{
std::vector<char> charBuffer;
std::vector<Color> textColorBuffer;
std::vector<Color> backgroundColorBuffer;
static const int frametime = 33;

public:

const size_t width;
const size_t height;

Framebuffer() :
width((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_col;
}()),
height((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_row;
}())
{
charBuffer = std::vector<char>(height*width);
textColorBuffer = std::vector<Color>(height*width);
backgroundColorBuffer = std::vector<Color>(height*width);
clear();
}
void clear()
{
for(auto& i : charBuffer)
{
i = ' ';
}
for(auto& i : textColorBuffer)
{
i = {255,255,255};
}
for(auto& i : backgroundColorBuffer)
{
i = {0,0,0};
}
}
void setChar(size_t col,size_t row, char c)
{
assert(row < height && col < width && row >= 0 && col >= 0);
charBuffer.at(row*width + col) = c;
}
void setChar(size_t col, size_t row, std::vector<std::string> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setChar(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setTextColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
textColorBuffer.at(row*width + col) = color;
}
void setTextColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setTextColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setBackgroundColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
backgroundColorBuffer.at(row*width + col) = color;
}
void setBackgroundColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setBackgroundColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
char getChar(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return charBuffer.at(row*width + col);
}
Color getTextColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return textColorBuffer.at(row*width + col);
}
Color getBackgroundColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return backgroundColorBuffer.at(row*width + col);
}
void print()
{
static std::thread printerThread;
if(printerThread.joinable())
{
printerThread.join();
}
auto printer = [this]()
{
std::string output = "";
for(size_t row = 0; row<height; row++)
{
for(size_t col = 0; col<width; col++)
{
Color textColor = getTextColor(col, row);
Color backgroundColor = getBackgroundColor(col, row);
output += "33[38;2;";
output += std::to_string((int)textColor.r) + ";";
output += std::to_string((int)textColor.g) + ";";
output += std::to_string((int)textColor.b) + "m";
output += "33[48;2;";
output += std::to_string((int)backgroundColor.r) + ";";
output += std::to_string((int)backgroundColor.g) + ";";
output += std::to_string((int)backgroundColor.b) + "m";
output += getChar(col, row);
}
if(row != height - 1)
{
output += "n";
}
}
std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
std::system("clear");
std::cout << output << std::flush;
};
printerThread = std::thread(printer);
}
};


There is a bug: when destructing a Framebuffer it can happen that the static std::thread printerThread never gets joined or otherwise terminated.










share|improve this question











$endgroup$








  • 1




    $begingroup$
    ncurses. Perhaps you've heard of it?
    $endgroup$
    – Reinderien
    23 hours ago










  • $begingroup$
    In setBackgroundColor, take std::vector<std::vector<Color>> by const-ref instead since you don't modify the object. The copy is unnecessary.
    $endgroup$
    – Juho
    15 hours ago
















7












$begingroup$


I am working on a pseudo graphical interface for a Chess engine I wrote. I want to draw a colored Chess board with ASCII pieces. To abstract the pure std::cout << std::endl; I wrote this little class to organize an ASCII-character "framebuffer":



#include <iostream>
#include <sys/ioctl.h>
#include <unistd.h>
#include <vector>
#include <string>
#include <cassert>
#include <chrono>
#include <thread>

struct Color
{
unsigned char r;
unsigned char g;
unsigned char b;
};

class Framebuffer
{
std::vector<char> charBuffer;
std::vector<Color> textColorBuffer;
std::vector<Color> backgroundColorBuffer;
static const int frametime = 33;

public:

const size_t width;
const size_t height;

Framebuffer() :
width((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_col;
}()),
height((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_row;
}())
{
charBuffer = std::vector<char>(height*width);
textColorBuffer = std::vector<Color>(height*width);
backgroundColorBuffer = std::vector<Color>(height*width);
clear();
}
void clear()
{
for(auto& i : charBuffer)
{
i = ' ';
}
for(auto& i : textColorBuffer)
{
i = {255,255,255};
}
for(auto& i : backgroundColorBuffer)
{
i = {0,0,0};
}
}
void setChar(size_t col,size_t row, char c)
{
assert(row < height && col < width && row >= 0 && col >= 0);
charBuffer.at(row*width + col) = c;
}
void setChar(size_t col, size_t row, std::vector<std::string> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setChar(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setTextColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
textColorBuffer.at(row*width + col) = color;
}
void setTextColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setTextColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setBackgroundColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
backgroundColorBuffer.at(row*width + col) = color;
}
void setBackgroundColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setBackgroundColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
char getChar(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return charBuffer.at(row*width + col);
}
Color getTextColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return textColorBuffer.at(row*width + col);
}
Color getBackgroundColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return backgroundColorBuffer.at(row*width + col);
}
void print()
{
static std::thread printerThread;
if(printerThread.joinable())
{
printerThread.join();
}
auto printer = [this]()
{
std::string output = "";
for(size_t row = 0; row<height; row++)
{
for(size_t col = 0; col<width; col++)
{
Color textColor = getTextColor(col, row);
Color backgroundColor = getBackgroundColor(col, row);
output += "33[38;2;";
output += std::to_string((int)textColor.r) + ";";
output += std::to_string((int)textColor.g) + ";";
output += std::to_string((int)textColor.b) + "m";
output += "33[48;2;";
output += std::to_string((int)backgroundColor.r) + ";";
output += std::to_string((int)backgroundColor.g) + ";";
output += std::to_string((int)backgroundColor.b) + "m";
output += getChar(col, row);
}
if(row != height - 1)
{
output += "n";
}
}
std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
std::system("clear");
std::cout << output << std::flush;
};
printerThread = std::thread(printer);
}
};


There is a bug: when destructing a Framebuffer it can happen that the static std::thread printerThread never gets joined or otherwise terminated.










share|improve this question











$endgroup$








  • 1




    $begingroup$
    ncurses. Perhaps you've heard of it?
    $endgroup$
    – Reinderien
    23 hours ago










  • $begingroup$
    In setBackgroundColor, take std::vector<std::vector<Color>> by const-ref instead since you don't modify the object. The copy is unnecessary.
    $endgroup$
    – Juho
    15 hours ago














7












7








7


2



$begingroup$


I am working on a pseudo graphical interface for a Chess engine I wrote. I want to draw a colored Chess board with ASCII pieces. To abstract the pure std::cout << std::endl; I wrote this little class to organize an ASCII-character "framebuffer":



#include <iostream>
#include <sys/ioctl.h>
#include <unistd.h>
#include <vector>
#include <string>
#include <cassert>
#include <chrono>
#include <thread>

struct Color
{
unsigned char r;
unsigned char g;
unsigned char b;
};

class Framebuffer
{
std::vector<char> charBuffer;
std::vector<Color> textColorBuffer;
std::vector<Color> backgroundColorBuffer;
static const int frametime = 33;

public:

const size_t width;
const size_t height;

Framebuffer() :
width((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_col;
}()),
height((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_row;
}())
{
charBuffer = std::vector<char>(height*width);
textColorBuffer = std::vector<Color>(height*width);
backgroundColorBuffer = std::vector<Color>(height*width);
clear();
}
void clear()
{
for(auto& i : charBuffer)
{
i = ' ';
}
for(auto& i : textColorBuffer)
{
i = {255,255,255};
}
for(auto& i : backgroundColorBuffer)
{
i = {0,0,0};
}
}
void setChar(size_t col,size_t row, char c)
{
assert(row < height && col < width && row >= 0 && col >= 0);
charBuffer.at(row*width + col) = c;
}
void setChar(size_t col, size_t row, std::vector<std::string> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setChar(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setTextColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
textColorBuffer.at(row*width + col) = color;
}
void setTextColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setTextColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setBackgroundColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
backgroundColorBuffer.at(row*width + col) = color;
}
void setBackgroundColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setBackgroundColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
char getChar(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return charBuffer.at(row*width + col);
}
Color getTextColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return textColorBuffer.at(row*width + col);
}
Color getBackgroundColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return backgroundColorBuffer.at(row*width + col);
}
void print()
{
static std::thread printerThread;
if(printerThread.joinable())
{
printerThread.join();
}
auto printer = [this]()
{
std::string output = "";
for(size_t row = 0; row<height; row++)
{
for(size_t col = 0; col<width; col++)
{
Color textColor = getTextColor(col, row);
Color backgroundColor = getBackgroundColor(col, row);
output += "33[38;2;";
output += std::to_string((int)textColor.r) + ";";
output += std::to_string((int)textColor.g) + ";";
output += std::to_string((int)textColor.b) + "m";
output += "33[48;2;";
output += std::to_string((int)backgroundColor.r) + ";";
output += std::to_string((int)backgroundColor.g) + ";";
output += std::to_string((int)backgroundColor.b) + "m";
output += getChar(col, row);
}
if(row != height - 1)
{
output += "n";
}
}
std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
std::system("clear");
std::cout << output << std::flush;
};
printerThread = std::thread(printer);
}
};


There is a bug: when destructing a Framebuffer it can happen that the static std::thread printerThread never gets joined or otherwise terminated.










share|improve this question











$endgroup$




I am working on a pseudo graphical interface for a Chess engine I wrote. I want to draw a colored Chess board with ASCII pieces. To abstract the pure std::cout << std::endl; I wrote this little class to organize an ASCII-character "framebuffer":



#include <iostream>
#include <sys/ioctl.h>
#include <unistd.h>
#include <vector>
#include <string>
#include <cassert>
#include <chrono>
#include <thread>

struct Color
{
unsigned char r;
unsigned char g;
unsigned char b;
};

class Framebuffer
{
std::vector<char> charBuffer;
std::vector<Color> textColorBuffer;
std::vector<Color> backgroundColorBuffer;
static const int frametime = 33;

public:

const size_t width;
const size_t height;

Framebuffer() :
width((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_col;
}()),
height((){
winsize w;
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
return w.ws_row;
}())
{
charBuffer = std::vector<char>(height*width);
textColorBuffer = std::vector<Color>(height*width);
backgroundColorBuffer = std::vector<Color>(height*width);
clear();
}
void clear()
{
for(auto& i : charBuffer)
{
i = ' ';
}
for(auto& i : textColorBuffer)
{
i = {255,255,255};
}
for(auto& i : backgroundColorBuffer)
{
i = {0,0,0};
}
}
void setChar(size_t col,size_t row, char c)
{
assert(row < height && col < width && row >= 0 && col >= 0);
charBuffer.at(row*width + col) = c;
}
void setChar(size_t col, size_t row, std::vector<std::string> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setChar(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setTextColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
textColorBuffer.at(row*width + col) = color;
}
void setTextColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setTextColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
void setBackgroundColor(size_t col,size_t row, Color color)
{
assert(row < height && col < width && row >= 0 && col >= 0);
backgroundColorBuffer.at(row*width + col) = color;
}
void setBackgroundColor(size_t col, size_t row, std::vector<std::vector<Color>> box)
{
assert(row < height && col < width && row >= 0 && col >= 0);
for(size_t rowOffset = 0; rowOffset<box.size(); rowOffset++)
{
for(size_t colOffset = 0; colOffset<box[rowOffset].size(); colOffset++)
{
setBackgroundColor(col+colOffset, row+rowOffset, box[rowOffset][colOffset]);
}
}
}
char getChar(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return charBuffer.at(row*width + col);
}
Color getTextColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return textColorBuffer.at(row*width + col);
}
Color getBackgroundColor(size_t col, size_t row)
{
assert(row < height && col < width && row >= 0 && col >= 0);
return backgroundColorBuffer.at(row*width + col);
}
void print()
{
static std::thread printerThread;
if(printerThread.joinable())
{
printerThread.join();
}
auto printer = [this]()
{
std::string output = "";
for(size_t row = 0; row<height; row++)
{
for(size_t col = 0; col<width; col++)
{
Color textColor = getTextColor(col, row);
Color backgroundColor = getBackgroundColor(col, row);
output += "33[38;2;";
output += std::to_string((int)textColor.r) + ";";
output += std::to_string((int)textColor.g) + ";";
output += std::to_string((int)textColor.b) + "m";
output += "33[48;2;";
output += std::to_string((int)backgroundColor.r) + ";";
output += std::to_string((int)backgroundColor.g) + ";";
output += std::to_string((int)backgroundColor.b) + "m";
output += getChar(col, row);
}
if(row != height - 1)
{
output += "n";
}
}
std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
std::system("clear");
std::cout << output << std::flush;
};
printerThread = std::thread(printer);
}
};


There is a bug: when destructing a Framebuffer it can happen that the static std::thread printerThread never gets joined or otherwise terminated.







c++ console linux ascii-art






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited 18 mins ago









Jamal

30.4k11121227




30.4k11121227










asked yesterday









Darius DuesentriebDarius Duesentrieb

24018




24018








  • 1




    $begingroup$
    ncurses. Perhaps you've heard of it?
    $endgroup$
    – Reinderien
    23 hours ago










  • $begingroup$
    In setBackgroundColor, take std::vector<std::vector<Color>> by const-ref instead since you don't modify the object. The copy is unnecessary.
    $endgroup$
    – Juho
    15 hours ago














  • 1




    $begingroup$
    ncurses. Perhaps you've heard of it?
    $endgroup$
    – Reinderien
    23 hours ago










  • $begingroup$
    In setBackgroundColor, take std::vector<std::vector<Color>> by const-ref instead since you don't modify the object. The copy is unnecessary.
    $endgroup$
    – Juho
    15 hours ago








1




1




$begingroup$
ncurses. Perhaps you've heard of it?
$endgroup$
– Reinderien
23 hours ago




$begingroup$
ncurses. Perhaps you've heard of it?
$endgroup$
– Reinderien
23 hours ago












$begingroup$
In setBackgroundColor, take std::vector<std::vector<Color>> by const-ref instead since you don't modify the object. The copy is unnecessary.
$endgroup$
– Juho
15 hours ago




$begingroup$
In setBackgroundColor, take std::vector<std::vector<Color>> by const-ref instead since you don't modify the object. The copy is unnecessary.
$endgroup$
– Juho
15 hours ago










2 Answers
2






active

oldest

votes


















2












$begingroup$


  • Order your includes at least by portable / non-portable.

  • Not a huge fan of omitting private and putting all the private members up top. IMO a class interface should go from public to private which makes for easier reading as a user.

  • The whole thing is a bit hard to read. Some linebreaks and maybe even spaces would make this easier on the eyes.

  • Is there a reason not to use memset in your clear function?

  • Pedantic people might complain about the missing header for size_t and the missing std:: qualifier.


  • std::string output = ""; initializing strings this way always looks weird to me. std::string s; should suffice but to declare intent more clearly you can do std::string{""};. Purely subjective though.

  • Always a good idea to get into the habit of using prefix operator over postfix operator.

  • I do like that you signal intent with flush as opposed to relying on endl

  • Not sure if you use Color elsewhere but it could probably be an implementation detail instead of being free.

  • You explicitly state this is for linux so you probably know that system("clear") is non-portable and are okay with it.






share|improve this answer









$endgroup$













  • $begingroup$
    #include <sys/ioctl.h> #include <unistd.h> are the non-portables, right?
    $endgroup$
    – Darius Duesentrieb
    yesterday










  • $begingroup$
    @DariusDuesentrieb As far as I can tell, yes.
    $endgroup$
    – yuri
    yesterday



















2












$begingroup$

Framebuffer()



Could end with



    charBuffer = std::vector<char>(height * width, ' ');
textColorBuffer = std::vector<Color>(height * width, {255u, 255u, 255u});
backgroundColorBuffer = std::vector<Color>(height * width);


instead of calling clear.



void clear()



Alternative implementation and let container implementation decide what is most effective.



    charBuffer.assign(charBuffer.size(), ' ');
textColorBuffer.assign(textColorBuffer.size(), {255u, 255u, 255u});
backgroundColorBuffer.assign(backgroundColorBuffer.size(), {});


void setChar()



Don't copy the box in the interface, use a const reference. And don't call size() more than necessary.



void setChar(size_t col, size_t row, const std::vector<std::string>& box) {
assert(row < height && col < width && row >= 0 && col >= 0);
for (size_t rowOffset = 0u, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
for (size_t colOffset = 0, rowSize = box[rowOffset].size(); colOffset < rowSize; colOffset++) {
setChar(col + colOffset, row + rowOffset, box[rowOffset][colOffset]);
}
}
}


void setTextColor()



Again, don't copy the box on each call. And another use of references inside the loops.



void setTextColor(size_t col, size_t row, const std::vector<std::vector<Color>>& box) {
assert(row < height && col < width && row >= 0 && col >= 0);
for (size_t rowOffset = 0, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
auto & line = box[rowOffset];
for (size_t colOffset = 0, line_sz = line.size(); colOffset < line_sz; colOffset++) {
setTextColor(col + colOffset, row + rowOffset, line[colOffset]);
}
}
}


void setBackgroundColor()



Similar comments regarding setBackgroundColor.



void print()



Alternative lambda with std::stringstream.



    auto printer = [this]() {
std::stringstream output;
for (size_t row = 0; row < height; row++) {
for (size_t col = 0; col < width; col++) {
Color textColor = getTextColor(col, row);
Color backgroundColor = getBackgroundColor(col, row);
output << "33[38;2;"
<< static_cast<int>(textColor.r) << ';'
<< static_cast<int>(textColor.g) << ';'
<< static_cast<int>(textColor.b) << "m"
"33[48;2;"
<< static_cast<int>(backgroundColor.r) << ';'
<< static_cast<int>(backgroundColor.g) << ';'
<< static_cast<int>(backgroundColor.b) << 'm'
<< getChar(col, row);
}
if (row != height - 1) {
output << 'n';
}
}
std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
std::system("clear");
std::cout << output.rdbuf() << std::flush;
};





share|improve this answer









$endgroup$













    Your Answer





    StackExchange.ifUsing("editor", function () {
    return StackExchange.using("mathjaxEditing", function () {
    StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
    StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
    });
    });
    }, "mathjax-editing");

    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%2f215519%2fcolor-ascii-drawing-class%23new-answer', 'question_page');
    }
    );

    Post as a guest















    Required, but never shown

























    2 Answers
    2






    active

    oldest

    votes








    2 Answers
    2






    active

    oldest

    votes









    active

    oldest

    votes






    active

    oldest

    votes









    2












    $begingroup$


    • Order your includes at least by portable / non-portable.

    • Not a huge fan of omitting private and putting all the private members up top. IMO a class interface should go from public to private which makes for easier reading as a user.

    • The whole thing is a bit hard to read. Some linebreaks and maybe even spaces would make this easier on the eyes.

    • Is there a reason not to use memset in your clear function?

    • Pedantic people might complain about the missing header for size_t and the missing std:: qualifier.


    • std::string output = ""; initializing strings this way always looks weird to me. std::string s; should suffice but to declare intent more clearly you can do std::string{""};. Purely subjective though.

    • Always a good idea to get into the habit of using prefix operator over postfix operator.

    • I do like that you signal intent with flush as opposed to relying on endl

    • Not sure if you use Color elsewhere but it could probably be an implementation detail instead of being free.

    • You explicitly state this is for linux so you probably know that system("clear") is non-portable and are okay with it.






    share|improve this answer









    $endgroup$













    • $begingroup$
      #include <sys/ioctl.h> #include <unistd.h> are the non-portables, right?
      $endgroup$
      – Darius Duesentrieb
      yesterday










    • $begingroup$
      @DariusDuesentrieb As far as I can tell, yes.
      $endgroup$
      – yuri
      yesterday
















    2












    $begingroup$


    • Order your includes at least by portable / non-portable.

    • Not a huge fan of omitting private and putting all the private members up top. IMO a class interface should go from public to private which makes for easier reading as a user.

    • The whole thing is a bit hard to read. Some linebreaks and maybe even spaces would make this easier on the eyes.

    • Is there a reason not to use memset in your clear function?

    • Pedantic people might complain about the missing header for size_t and the missing std:: qualifier.


    • std::string output = ""; initializing strings this way always looks weird to me. std::string s; should suffice but to declare intent more clearly you can do std::string{""};. Purely subjective though.

    • Always a good idea to get into the habit of using prefix operator over postfix operator.

    • I do like that you signal intent with flush as opposed to relying on endl

    • Not sure if you use Color elsewhere but it could probably be an implementation detail instead of being free.

    • You explicitly state this is for linux so you probably know that system("clear") is non-portable and are okay with it.






    share|improve this answer









    $endgroup$













    • $begingroup$
      #include <sys/ioctl.h> #include <unistd.h> are the non-portables, right?
      $endgroup$
      – Darius Duesentrieb
      yesterday










    • $begingroup$
      @DariusDuesentrieb As far as I can tell, yes.
      $endgroup$
      – yuri
      yesterday














    2












    2








    2





    $begingroup$


    • Order your includes at least by portable / non-portable.

    • Not a huge fan of omitting private and putting all the private members up top. IMO a class interface should go from public to private which makes for easier reading as a user.

    • The whole thing is a bit hard to read. Some linebreaks and maybe even spaces would make this easier on the eyes.

    • Is there a reason not to use memset in your clear function?

    • Pedantic people might complain about the missing header for size_t and the missing std:: qualifier.


    • std::string output = ""; initializing strings this way always looks weird to me. std::string s; should suffice but to declare intent more clearly you can do std::string{""};. Purely subjective though.

    • Always a good idea to get into the habit of using prefix operator over postfix operator.

    • I do like that you signal intent with flush as opposed to relying on endl

    • Not sure if you use Color elsewhere but it could probably be an implementation detail instead of being free.

    • You explicitly state this is for linux so you probably know that system("clear") is non-portable and are okay with it.






    share|improve this answer









    $endgroup$




    • Order your includes at least by portable / non-portable.

    • Not a huge fan of omitting private and putting all the private members up top. IMO a class interface should go from public to private which makes for easier reading as a user.

    • The whole thing is a bit hard to read. Some linebreaks and maybe even spaces would make this easier on the eyes.

    • Is there a reason not to use memset in your clear function?

    • Pedantic people might complain about the missing header for size_t and the missing std:: qualifier.


    • std::string output = ""; initializing strings this way always looks weird to me. std::string s; should suffice but to declare intent more clearly you can do std::string{""};. Purely subjective though.

    • Always a good idea to get into the habit of using prefix operator over postfix operator.

    • I do like that you signal intent with flush as opposed to relying on endl

    • Not sure if you use Color elsewhere but it could probably be an implementation detail instead of being free.

    • You explicitly state this is for linux so you probably know that system("clear") is non-portable and are okay with it.







    share|improve this answer












    share|improve this answer



    share|improve this answer










    answered yesterday









    yuriyuri

    3,63921034




    3,63921034












    • $begingroup$
      #include <sys/ioctl.h> #include <unistd.h> are the non-portables, right?
      $endgroup$
      – Darius Duesentrieb
      yesterday










    • $begingroup$
      @DariusDuesentrieb As far as I can tell, yes.
      $endgroup$
      – yuri
      yesterday


















    • $begingroup$
      #include <sys/ioctl.h> #include <unistd.h> are the non-portables, right?
      $endgroup$
      – Darius Duesentrieb
      yesterday










    • $begingroup$
      @DariusDuesentrieb As far as I can tell, yes.
      $endgroup$
      – yuri
      yesterday
















    $begingroup$
    #include <sys/ioctl.h> #include <unistd.h> are the non-portables, right?
    $endgroup$
    – Darius Duesentrieb
    yesterday




    $begingroup$
    #include <sys/ioctl.h> #include <unistd.h> are the non-portables, right?
    $endgroup$
    – Darius Duesentrieb
    yesterday












    $begingroup$
    @DariusDuesentrieb As far as I can tell, yes.
    $endgroup$
    – yuri
    yesterday




    $begingroup$
    @DariusDuesentrieb As far as I can tell, yes.
    $endgroup$
    – yuri
    yesterday













    2












    $begingroup$

    Framebuffer()



    Could end with



        charBuffer = std::vector<char>(height * width, ' ');
    textColorBuffer = std::vector<Color>(height * width, {255u, 255u, 255u});
    backgroundColorBuffer = std::vector<Color>(height * width);


    instead of calling clear.



    void clear()



    Alternative implementation and let container implementation decide what is most effective.



        charBuffer.assign(charBuffer.size(), ' ');
    textColorBuffer.assign(textColorBuffer.size(), {255u, 255u, 255u});
    backgroundColorBuffer.assign(backgroundColorBuffer.size(), {});


    void setChar()



    Don't copy the box in the interface, use a const reference. And don't call size() more than necessary.



    void setChar(size_t col, size_t row, const std::vector<std::string>& box) {
    assert(row < height && col < width && row >= 0 && col >= 0);
    for (size_t rowOffset = 0u, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
    for (size_t colOffset = 0, rowSize = box[rowOffset].size(); colOffset < rowSize; colOffset++) {
    setChar(col + colOffset, row + rowOffset, box[rowOffset][colOffset]);
    }
    }
    }


    void setTextColor()



    Again, don't copy the box on each call. And another use of references inside the loops.



    void setTextColor(size_t col, size_t row, const std::vector<std::vector<Color>>& box) {
    assert(row < height && col < width && row >= 0 && col >= 0);
    for (size_t rowOffset = 0, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
    auto & line = box[rowOffset];
    for (size_t colOffset = 0, line_sz = line.size(); colOffset < line_sz; colOffset++) {
    setTextColor(col + colOffset, row + rowOffset, line[colOffset]);
    }
    }
    }


    void setBackgroundColor()



    Similar comments regarding setBackgroundColor.



    void print()



    Alternative lambda with std::stringstream.



        auto printer = [this]() {
    std::stringstream output;
    for (size_t row = 0; row < height; row++) {
    for (size_t col = 0; col < width; col++) {
    Color textColor = getTextColor(col, row);
    Color backgroundColor = getBackgroundColor(col, row);
    output << "33[38;2;"
    << static_cast<int>(textColor.r) << ';'
    << static_cast<int>(textColor.g) << ';'
    << static_cast<int>(textColor.b) << "m"
    "33[48;2;"
    << static_cast<int>(backgroundColor.r) << ';'
    << static_cast<int>(backgroundColor.g) << ';'
    << static_cast<int>(backgroundColor.b) << 'm'
    << getChar(col, row);
    }
    if (row != height - 1) {
    output << 'n';
    }
    }
    std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
    std::system("clear");
    std::cout << output.rdbuf() << std::flush;
    };





    share|improve this answer









    $endgroup$


















      2












      $begingroup$

      Framebuffer()



      Could end with



          charBuffer = std::vector<char>(height * width, ' ');
      textColorBuffer = std::vector<Color>(height * width, {255u, 255u, 255u});
      backgroundColorBuffer = std::vector<Color>(height * width);


      instead of calling clear.



      void clear()



      Alternative implementation and let container implementation decide what is most effective.



          charBuffer.assign(charBuffer.size(), ' ');
      textColorBuffer.assign(textColorBuffer.size(), {255u, 255u, 255u});
      backgroundColorBuffer.assign(backgroundColorBuffer.size(), {});


      void setChar()



      Don't copy the box in the interface, use a const reference. And don't call size() more than necessary.



      void setChar(size_t col, size_t row, const std::vector<std::string>& box) {
      assert(row < height && col < width && row >= 0 && col >= 0);
      for (size_t rowOffset = 0u, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
      for (size_t colOffset = 0, rowSize = box[rowOffset].size(); colOffset < rowSize; colOffset++) {
      setChar(col + colOffset, row + rowOffset, box[rowOffset][colOffset]);
      }
      }
      }


      void setTextColor()



      Again, don't copy the box on each call. And another use of references inside the loops.



      void setTextColor(size_t col, size_t row, const std::vector<std::vector<Color>>& box) {
      assert(row < height && col < width && row >= 0 && col >= 0);
      for (size_t rowOffset = 0, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
      auto & line = box[rowOffset];
      for (size_t colOffset = 0, line_sz = line.size(); colOffset < line_sz; colOffset++) {
      setTextColor(col + colOffset, row + rowOffset, line[colOffset]);
      }
      }
      }


      void setBackgroundColor()



      Similar comments regarding setBackgroundColor.



      void print()



      Alternative lambda with std::stringstream.



          auto printer = [this]() {
      std::stringstream output;
      for (size_t row = 0; row < height; row++) {
      for (size_t col = 0; col < width; col++) {
      Color textColor = getTextColor(col, row);
      Color backgroundColor = getBackgroundColor(col, row);
      output << "33[38;2;"
      << static_cast<int>(textColor.r) << ';'
      << static_cast<int>(textColor.g) << ';'
      << static_cast<int>(textColor.b) << "m"
      "33[48;2;"
      << static_cast<int>(backgroundColor.r) << ';'
      << static_cast<int>(backgroundColor.g) << ';'
      << static_cast<int>(backgroundColor.b) << 'm'
      << getChar(col, row);
      }
      if (row != height - 1) {
      output << 'n';
      }
      }
      std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
      std::system("clear");
      std::cout << output.rdbuf() << std::flush;
      };





      share|improve this answer









      $endgroup$
















        2












        2








        2





        $begingroup$

        Framebuffer()



        Could end with



            charBuffer = std::vector<char>(height * width, ' ');
        textColorBuffer = std::vector<Color>(height * width, {255u, 255u, 255u});
        backgroundColorBuffer = std::vector<Color>(height * width);


        instead of calling clear.



        void clear()



        Alternative implementation and let container implementation decide what is most effective.



            charBuffer.assign(charBuffer.size(), ' ');
        textColorBuffer.assign(textColorBuffer.size(), {255u, 255u, 255u});
        backgroundColorBuffer.assign(backgroundColorBuffer.size(), {});


        void setChar()



        Don't copy the box in the interface, use a const reference. And don't call size() more than necessary.



        void setChar(size_t col, size_t row, const std::vector<std::string>& box) {
        assert(row < height && col < width && row >= 0 && col >= 0);
        for (size_t rowOffset = 0u, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
        for (size_t colOffset = 0, rowSize = box[rowOffset].size(); colOffset < rowSize; colOffset++) {
        setChar(col + colOffset, row + rowOffset, box[rowOffset][colOffset]);
        }
        }
        }


        void setTextColor()



        Again, don't copy the box on each call. And another use of references inside the loops.



        void setTextColor(size_t col, size_t row, const std::vector<std::vector<Color>>& box) {
        assert(row < height && col < width && row >= 0 && col >= 0);
        for (size_t rowOffset = 0, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
        auto & line = box[rowOffset];
        for (size_t colOffset = 0, line_sz = line.size(); colOffset < line_sz; colOffset++) {
        setTextColor(col + colOffset, row + rowOffset, line[colOffset]);
        }
        }
        }


        void setBackgroundColor()



        Similar comments regarding setBackgroundColor.



        void print()



        Alternative lambda with std::stringstream.



            auto printer = [this]() {
        std::stringstream output;
        for (size_t row = 0; row < height; row++) {
        for (size_t col = 0; col < width; col++) {
        Color textColor = getTextColor(col, row);
        Color backgroundColor = getBackgroundColor(col, row);
        output << "33[38;2;"
        << static_cast<int>(textColor.r) << ';'
        << static_cast<int>(textColor.g) << ';'
        << static_cast<int>(textColor.b) << "m"
        "33[48;2;"
        << static_cast<int>(backgroundColor.r) << ';'
        << static_cast<int>(backgroundColor.g) << ';'
        << static_cast<int>(backgroundColor.b) << 'm'
        << getChar(col, row);
        }
        if (row != height - 1) {
        output << 'n';
        }
        }
        std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
        std::system("clear");
        std::cout << output.rdbuf() << std::flush;
        };





        share|improve this answer









        $endgroup$



        Framebuffer()



        Could end with



            charBuffer = std::vector<char>(height * width, ' ');
        textColorBuffer = std::vector<Color>(height * width, {255u, 255u, 255u});
        backgroundColorBuffer = std::vector<Color>(height * width);


        instead of calling clear.



        void clear()



        Alternative implementation and let container implementation decide what is most effective.



            charBuffer.assign(charBuffer.size(), ' ');
        textColorBuffer.assign(textColorBuffer.size(), {255u, 255u, 255u});
        backgroundColorBuffer.assign(backgroundColorBuffer.size(), {});


        void setChar()



        Don't copy the box in the interface, use a const reference. And don't call size() more than necessary.



        void setChar(size_t col, size_t row, const std::vector<std::string>& box) {
        assert(row < height && col < width && row >= 0 && col >= 0);
        for (size_t rowOffset = 0u, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
        for (size_t colOffset = 0, rowSize = box[rowOffset].size(); colOffset < rowSize; colOffset++) {
        setChar(col + colOffset, row + rowOffset, box[rowOffset][colOffset]);
        }
        }
        }


        void setTextColor()



        Again, don't copy the box on each call. And another use of references inside the loops.



        void setTextColor(size_t col, size_t row, const std::vector<std::vector<Color>>& box) {
        assert(row < height && col < width && row >= 0 && col >= 0);
        for (size_t rowOffset = 0, boxSize = box.size(); rowOffset < boxSize; rowOffset++) {
        auto & line = box[rowOffset];
        for (size_t colOffset = 0, line_sz = line.size(); colOffset < line_sz; colOffset++) {
        setTextColor(col + colOffset, row + rowOffset, line[colOffset]);
        }
        }
        }


        void setBackgroundColor()



        Similar comments regarding setBackgroundColor.



        void print()



        Alternative lambda with std::stringstream.



            auto printer = [this]() {
        std::stringstream output;
        for (size_t row = 0; row < height; row++) {
        for (size_t col = 0; col < width; col++) {
        Color textColor = getTextColor(col, row);
        Color backgroundColor = getBackgroundColor(col, row);
        output << "33[38;2;"
        << static_cast<int>(textColor.r) << ';'
        << static_cast<int>(textColor.g) << ';'
        << static_cast<int>(textColor.b) << "m"
        "33[48;2;"
        << static_cast<int>(backgroundColor.r) << ';'
        << static_cast<int>(backgroundColor.g) << ';'
        << static_cast<int>(backgroundColor.b) << 'm'
        << getChar(col, row);
        }
        if (row != height - 1) {
        output << 'n';
        }
        }
        std::this_thread::sleep_for(std::chrono::milliseconds(frametime));
        std::system("clear");
        std::cout << output.rdbuf() << std::flush;
        };






        share|improve this answer












        share|improve this answer



        share|improve this answer










        answered 19 hours ago









        Bo RBo R

        1611




        1611






























            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%2f215519%2fcolor-ascii-drawing-class%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

            Сан-Квентин

            8-я гвардейская общевойсковая армия

            Алькесар