Text-Based Simulator with many commands

Question

The following is an excerpt of a mostly text-based simulator of the iRobot Roomba robot vacuum. I made a class called Roomba that handles everything the roomba does. For those familiar with QT; There is also a class called MainWindow that handles the GUI. This is not that class.

The roomba has several commands that I defined partly in structs, and partly in seperate voids. There's quite a few commands and the way I currently have the code laid out (see below) feels very inefficient. commands have an OpCode (128-150) and a name. Some commands have databytes, others do not. Commands are available in one or more operating modes (differs per command).All commands print regular messages and error messages, though some commands are similar and print similar messages.

I'm currently using a switch, that redirects to the appropraite void, which draws data about the command from structs. The program currently works as intended, But I want to make the code a little better.

My question is: How can I optimize the following code, so that I can easily add more commands, without obfuscating the code. Other tips and comments are also welcome, of course.

Here's the excerpt, simplified to only two commands; 'Start' and 'Baud'. 'Start' starts the roomba, 'Baud' sets the baudrate code (0-11, 11 by default). 'Start' is available in all operating modes, and changes the operating mode to 'Passive'. 'Baud' is available in all Operating Modes except for 'Off'. 'Start' required no databits, 'Baud' requires one. ROOMBA_START and ROOMBA_BAUD are defined in roomba.h as 128 and 129 respectively.

I try to maintain the Allman style and lowerCamelCasing.

roomba.cpp

#include "roomba.h"

/* OpCode Structs */
struct roombaCommand {
    QString commandName;
    int opCode;
    int dataBytes;
};

roombaCommand cmdStart,
              cmdBaud,

Roomba::Roomba()
{
    setRoombaToDefaultValues();
    initRoombaCommands();
}

Roomba::~Roomba()
{

}

QString Roomba::getError()
{
    return roombaErrorMessage;
}

void Roomba::setError(int32_t error_tmp = 0, QString opcode_tmp = "")
{
    roombaErrorNumber = error_tmp;

    switch(roombaErrorNumber){
        default: roombaErrorMessage =
                "An error ocurred, but no error message was specified.";
                break;
        case 1: roombaErrorMessage =
                "An unknown error ocurred.";
                break;
        case 2: roombaErrorMessage =
                "Invalid number op parameters for OpCode \"" + opcode_tmp + "\".";
                break;
        case 3: roombaErrorMessage =
                "Invalid baudcode specified for OpCode \"Baud\". Only the values 0-11 are accepted.";
                break;
        case 4: roombaErrorMessage =
                "Invalid operating mode for opcode \"" + opcode_tmp + "\". OpCode not availabe in " + roombaOperatingMode + " mode.";
                break;
    }

}

QString Roomba::getMessage()
{
    return roombaMessage;
}

void Roomba::setMessage(int32_t message_tmp = 0, QString parameter_tmp = "")
{
    switch(message_tmp){
        default: roombaMessage =
                "";
                break;
        case 1: roombaMessage =
                "Changed roomba operating mode to \"" + parameter_tmp + "\".";
                break;
        case 2: roombaMessage =
                "Roomba operating mode was already set to \"" + parameter_tmp + "\". Operating mode remains unchanged.";
                break;
        case 3: roombaMessage =
                "Changed roomba baudcode to \"" + parameter_tmp + "\".";
                break;
        case 4: roombaMessage =
                "Roomba baudcode was already set to \"" + parameter_tmp + "\". Baudcode remains unchanged.";
                break;
    }

}

int16_t Roomba::getRoombaBaudCode()
{
    return roombaBaudCode;
}

void Roomba::setRoombaBaudCode(int16_t baudCode)
{
    if (baudCode >= 0 && baudCode <= 11)
    {
       roombaBaudCode = baudCode;
    }
    else
    {
       setError(3);
    }
}

QString Roomba::getRoombaOperatingMode()
{
    return roombaOperatingMode;
}

void Roomba::setRoombaOperatingMode(QString modeName)
{
    if (   modeName == "Off"
        || modeName == "Passive"
        || modeName == "Safe"
        || modeName == "Full" )
    {
        Roomba::roombaOperatingMode = modeName;
    }
}

int16_t Roomba::getOpCode()
{
    return roombaCurrentOpCodeValue;
}

void Roomba::setOpCode(QString receivedCommandParameters)
{
    QStringList params_temp = receivedCommandParameters.split(",");
    int opCode_tmp = params_temp[0].toInt();

    switch(opCode_tmp){

        /* Unknown OpCode (-1) */
        default:
            roombaCurrentOpCodeName = "Unknown";
            roombaCurrentOpCodeValue = -1;
            break;

        case ROOMBA_START:

            if( params_temp.size() == (cmdStart.dataBytes + 1) )
            {
                opCode_Start();
            }
            else
            {
                setError(2, cmdStart.commandName);
            }
            break;

        case ROOMBA_BAUD:

            if( params_temp.size() == (cmdBaud.dataBytes + 1) )
            {
                opCode_Baud(params_temp[1].toInt());
            }
            else
            {
                setError(2, cmdBaud.commandName);
            }
            break;
    }
}

void Roomba::opCode_Start()
{
    roombaCurrentOpCodeName = cmdStart.commandName;
    roombaCurrentOpCodeValue = cmdStart.opCode;
    if(roombaOperatingMode == "Off")
    {
        setMessage(1, "Passive");
    }
    else
    {
        setMessage(2, "Passive");
    }
    setRoombaOperatingMode("Passive");

}

void Roomba::opCode_Baud(int16_t baudCode_tmp)
{
    if(roombaOperatingMode != "Off")
    {
        roombaCurrentOpCodeName = cmdBaud.commandName;
        roombaCurrentOpCodeValue = cmdBaud.opCode;
        if(roombaBaudCode != baudCode_tmp)
        {
             setMessage(3, QString::number(baudCode_tmp));
        }
        else
        {
            setMessage(4, QString::number(baudCode_tmp));
        }
        setRoombaBaudCode(baudCode_tmp);
    }
    else
    {
        setError(4, cmdBaud.commandName);
    }
}
        
void Roomba::initRoombaCommands(){
    // Set Commands
    cmdStart.commandName    = "Start";
    cmdStart.opCode         = ROOMBA_START;
    cmdStart.dataBytes      = 0;

    cmdBaud.commandName     = "Baud";
    cmdBaud.opCode          = ROOMBA_BAUD;
    cmdBaud.dataBytes       = 1;
}

void Roomba::setRoombaToDefaultValues(){
    // Set Default Values
    roombaBaudCode           =  11;         // Baudrate: 115200 Baud
    roombaOperatingMode      =  "Off";      // Mode:     Safe
    roombaCurrentOpCodeName  =  "";         // OpCode:   None (Idle)
    roombaCurrentOpCodeValue =  -1;         // OpCode:   -1   (None)
    roombaErrorNumber        =  0;          // Error Code 0, no error
    roombaMessage            =  "";         // No Message
}

Answer 1

By "optimizing" you do not mean speed (as most do), but readability/maintainability. Thats good.

You could create an array of function pointers where you store the actual voids, for example (this is dry-coded, could contain errors):

// somewhere in constructor
typedef (void) tcommandfunc(argument_type);    // that typedef is probably wrong
tvoidfunc commands[N_OPCODES];
commands[ROOMBA_START] = &rombaStart;
commands[ROOMBA_BAUD] = &rombaBaud;

// in Roomba::setOpCode
commands[opCode_tmp](arguments);

You would need to make sure that every entry has an actual funtion, for example the default function. Otherwise this will crash at runtime. Also you need to check that the opcode is inside the range of the array.

Another idea would be to use a class for commands. This class could have an execute method. Still you would need to map the opcode to the class.

C/C++ allows you to split the implementation in different files. Maybe for larger opcodes an individual file can help keeping the code manageable.