Mover font includes al .h
Algunos comentario son inutiles, cosas como esto no tienen sentido:
//Initialization of the static vars.
DeberíasMover definición de constantes al .h
No tiene sentido tener bloques de comentarios cerrados con cosas así en todas partes:
/**************************************/
El método Init debería recibir una estructura u objeto para no tener que pasar varios parametros según diferentes circunstancias.
En C++ usualmente no se utiliza tener nombre métodos capitalizados. En vez de SetFontColor se
usa set_font_color. De ahora en adelante voy a seguir usando tu estilo, aunque no crea en él.
En SetFontColor estás usando polimorfismo para algo que realmente debería ser:
void PS3Printer::SetColor(int fgColor, int bgColor)
void PS3Printer::SetForegroundColor(int fgColor)
void PS3Printer::SetBackgroundColor(int bgColor)
Usar variables _foo en los parametros de un método no tiene sentido sino tenés una propiedad
con el mismo nombre en ese mismo scope. Por ejemplo:
void PS3Printer::SetFontColor(int _fgColor) {
sconsole.__bgColor = FONT_COLOR_NONE;
sconsole.__fgColor = _fgColor;
}
Podría ser reemplazado por:
void PS3Printer::SetFontColor(int fgColor) {
sconsole.__bgColor = FONT_COLOR_NONE;
sconsole.__fgColor = fgColor;
}
Que es más legible. Se usa _fgColor cuando existe otra variable llamada fgColor en ese mismo
scope (o en el global), por ejemplo:
void PS3Printer::SetFontColor(int _fgColor) {
fgColor = _fgcolor; // aqui sí tiene sentido usar eso
}
En esta definición de método:
uint32_t PS3Printer::CalculateAlpha(const uint32_t &src, const uint32_t &bg) {//bg -> Background; src -> Color we want to calculate-alpha
Estás usando comentarios para algo que un código más legible podría resolver:
uint32_t PS3Printer::CalculateAlphaForColor(const uint32_t &color, const uint32_t &bg_color) {
Lo repetís en la línea siguiente:
uint32_t a = src >> 24; // alpha
Podrías hacer simplemente:
uint32_t alpha = src >> 24;
El nombre de las variables no te cuestan más si son más largas, no tengás miedo de usar nombres descriptivos.
Deberías usar {} en todos los condicionales:
if (0 == a)
return bg;
if(0xff == a)
return src;
La razón es simple: si el día de mañana tenés que hacer otra cosa antes del return te vas a
equivocar si se te olvida que no tiene {}.
Si usaras orientación a objetos en vez de números a la fuerza podrías limpiar el código:
if (0 == a)
return bg;
if(0xff == a)
return src;
Orientado a objetos (suponiendo que alpha es un objeto):
if (alpha.isEmpty())
return bg;
else if(alpha.isFullyTransparent())
return src;
En SetFontValue podrías recibir un objeto o struct en vez de 6 parametros. Teniendo un objeto
podes tener un metodo que se llame toSConsole o algo así que te permita escribir el método como:
void PS3Printer::SetFontValues(FontValue values) {
values.toSConsole(sconsole);
}
Lo mismo aplica en SetFontValuesMonospaced.
Debes usar nombres más descriptivos en los métodos:
void PS3Printer::SetDefaultValues();
Podría ser facilmente:
void PS3Printer::SetDefaultFont();
El método Print está haciendo demasiadas cosas. Podés ignorar el resto de este documento
pero tenés que cambiar ese método. Lo que hay después del for simplemente es imposible de
leer. Tenés que partir eso en varios métodos.
Seguís con los comentarios inutiles:
SetFontColor(FONT_COLOR_RED, FONT_COLOR_YELLOW);//red background, yellow letters
Leyendo el código podría ver los colores, no necesito el comentario para recordarmelo. Además,
considerá usar COLOR_RED o RED en vez de FONT_COLOR_RED ya que ese color sirve para cosas
adicionales a las fuentes.
Deberías tener una variable en la clase con el valor de buffer para que no lo tengás que pasar
en todos los diferentes métodos print.
El espacio en blanco después de PrintWarning simplemente no tiene disculpa.
El método SetFont es ridículo, usa objetos y no te pongas a hacer case por propiedades.