Console class that handles multiple buffers

I'm working on a 80x25 terminal that splits the screen into two. The top half gets the first buffer, the bottom half gets the second buffer. I'm concerned about readability and accessibility.

For accessibility, what I'm trying to do is hide as much of the implementation as possible so that other parts of the code don't have to rely on this class as much. For example, I'm able to plug in a third party printf implementation and only have to change it at one place, where it outputs the buffer. Therefore 90% of the class should be private. The user only uses printf for outputting, unless they need to change the buffer or color.

For readability, after adding the logic to handle two buffers, I noticed an explosion in buffers[current_buffer]. I want to be able to reduce this. As well as that, I've found myself making typos such as typing 'rows' instead of 'columns' and so on. By tackling readability, annoying bugs like this hopefully can be eliminated.

Should I make terminal a global object?

Terminal.hpp

extern Terminal terminal;

Terminal.cpp

// Define terminal in one translation unit.
Terminal terminal;

Example usage:

main.cpp

printf("Buffer 1.\n");
// Zero-indexed.
terminal.setbuffer(1);
printf("Buffer 2.\n");

printf.cpp

// Printf doesn't need to care about terminal's internals. Just copy
// to buffer and output result to screen.
void printf(const char* fmt, ...)
{
    va_list args;
    int i;
    va_start(args, fmt);
    i = vsprintf(terminal.getcurrentbuffer().buffer, fmt, args);
    va_end(args);
    terminal.getcurrentbuffer().buffer[i] = '\0';
    terminal.puts(terminal.getcurrentbuffer().buffer);
}

How should I handle access to the current buffer and setting it?

terminal.getcurrentbuffer().buffer is an eyesore and a handful to type. But I don't want to actually return a char* instead of a Buffer&. Secondly, terminal.puts(terminal.getcurrentbuffer().buffer); seems rather redundant, but currently I just write

Should I make terminal a global object?

No. This is an implementation detail and you (probably) don't want to impose this decision on client code; That is, client code should be able to decide at any point if application needs a global, a temporary or multiple instances used for various situations.

Other remarks:

terminal.setbuffer(1);

This is not setting a buffer, but selecting a buffer by index (consider the names selectbuffer, choosebuffer, setbufferindex and so on).

Terminal::Buffer is a structure. Consider creating it as a class (outside of Terminal) and not exposing it's data.

Currently you expose it's data to client code and (because of it) you end up with this client code:

terminal.getcurrentbuffer().buffer
//      ^^^^^^^^^^^^^^^^^^^^^^^^^^ <- should be implementation detail of terminal
// and not exposed publicly

for (int i = buffers[current_buffer].ybase [...] )
//           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ should be behind an iterator
// or class interface

but currently I just write directly to video memory.

Don't! The only scenario when you should expose the private data of a class is when the class stays valid no matter what client code does with the data (in other words, if I cannot memcpy 2 Gb of random data (for example) into the public buffer member of Terminal::Buffer, it should not be public. You should instead add an API to write to the buffer (and this API should check boundaries, validate contents and so on).


How should I handle access to the current buffer and setting it?

You should not give access to the current buffer. You should instead expose an API that allows client code to add data to the terminal. If client code needs to know there are buffers, client code should choose the buffer in the write operation.

Old code:

void printf(const char* fmt, ...)
{
    va_list args;
    int i;
    va_start(args, fmt);
    i = vsprintf(terminal.getcurrentbuffer().buffer, fmt, args);
    va_end(args);
    terminal.getcurrentbuffer().buffer[i] = '\0';
    terminal.puts(terminal.getcurrentbuffer().buffer);
}

New code:

void printf(const char* fmt, ...)
{
    char localBuffer[1024]; // this is not very flexible
    va_list args;
    int i;
    va_start(args, fmt);
    i = vsprintf(localBuffer, fmt, args);
    va_end(args);
    terminal.write(localBuffer, i); // << you should implement this
}

Your Terminal::write API shoud:

• write to the current buffer (up to n characters)

• manage zero termination

• handle (or throw) errors

This API does not require client code to know about buffers.

StackExchange Code Review Q#63225, answer score: 5