Search code examples
c++cswitch-statementreusability

Making a switch-heavy function more reusable


Suppose I have this code:

void ClassA::SomeFunction()
{
    switch(something)
    {
    case case1:
        doSomething();
        break;
    case case2:
        doSomethingElse();
        break;
       .
       . 
       .
}
...
void ClassB::SomeFunction()
{
    switch(something) // this 'something' is the same 'something' as in
    {                 // the above class
    case case1:
        doSomethingCompletelyUnrelatedToClassAFunction();
        break;
    case case2:
        doSomethingCompletelyUnrelatedToClassAOtherFunction();
        break;
       .
       . 
       .
}  

The two functions in two classes do fairly different things under the exact same cases (all cases within both switches are exactly the same). Basically I'm writing a small Chip-8 emulator for fun and the two classes represent my "CPU" and disassembler.

The CPU should decode the opcode and do some stuff while the disassembler should simply format a string based on the opcode.

I'm trying to think of a way to avoid copy/pasting the entire switch and just changing the functions called in each case.

One simple solution I've come up with is make an abstract class which handles an opcode and has a different method for each of the cases in the switch. Both the CPU and the Disassembler classes would extend this class and implement their own behavior for each of the methods. However I'm hoping for a more elegant solution than that.

Thanks.

EDIT:

As per @Dogbert 's comment I'm adding a chunk of actual code:

wxString* DebugWindow::DisassembleOpCode(uint16_t opCode)
{
wxString disLine;

uint8_t nibble1 = (opCode & 0xF000) >> 12;
uint8_t nibble2 = (opCode & 0x0F00) >> 8;
uint8_t nibble3 = (opCode & 0x00F0) >> 4;
uint8_t nibble4 = (opCode & 0x000F);

if (opCode == 0x00E0)
    disLine = "CLS";
else if (opCode == 0x00EE)
    disLine = "RET";
else
    switch (nibble1)
{
    case 0x1:
    {
                // JMP nnn
                disLine = wxString::Format("JMP %04x", nibble2 | nibble3 | nibble4);
                break;
    }
    case 0x2:
    {
                // CALL nnn
                disLine = wxString::Format("CALL %04x", nibble2 | nibble3 | nibble4);
                break;
    }
    case 0x3:
    {
                // SE V[x], nn  -- skip next instruction if V[x] == nn
                disLine = wxString::Format("SE V[%x], %02x", nibble2, nibble3 | nibble4);
                break;
    }

    ...
    ...
    ...

    case 0x8: // arithmetic operations between registers, see below
        {
                      switch (nibble4)
                      {
                      case 0x0:
                      {
                                  // LD V[x], V[y] -- sets value of register V[x] to value of register V[y]
                                  disLine = wxString::Format("ADD V[%x], V[%x]", nibble2, nibble3);
                                  break;
                      }
                      case 0x1:
                      {
                                  // OR V[x], V[y] -- performs bitwise OR of values of registers V[x] and V[y], the result is stored in V[x]
                                  disLine = wxString::Format("OR V[%x], V[%x]", nibble2, nibble3);
                                  break;
                      }

     ...
     ...

This is a part of DisassembleOpcode function. The Step function in the CPU class should actually do stuff instead of formatting a string but is consisted of the same switch/cases.


Solution

  • In the same vein as dasblinkenlight suggested, I would even say that "tabulizing" the instruction-set would greatly enhance the readability of the design. Something along the lines of:

    struct opcode instructionset[] = {
    { 0x1000, "JMP %04x", &do_jump},
    ...
    

    Your code would then access the table for each instruction, probably the table would be sorted by opcode for quick lookup:

    struct opcode op* = find(opCode, instructionset);
    // for printing the instruction
    disLine = wxString::Format(op->formatstring, opCode & op->format_mask);
    

    You get the idea... But the key point is: You would have a nicely formatted table of your implemented instructionset, which I think would greatly ease maintenance / future addition of instructions.