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.

• 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.

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;

    };