Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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.

like image 313
blashyrk Avatar asked Aug 23 '26 09:08

blashyrk


1 Answers

In C, you can replace a switch with an array of function pointers. In C++ you have an additional option of using pointers to a class with polymorphic behavior.

Here is how you can do it with function pointers:

typedef void (*disassembly_action)(uint8_t,uint8_t,uint8_t,uint8_t);

static void doSomethingOpcodeZero(uint8_t,uint8_t,uint8_t,uint8_t);
static void doSomethingOpcodeOne(uint8_t,uint8_t,uint8_t,uint8_t);
static void doSomethingOpcodeTwo(uint8_t,uint8_t,uint8_t,uint8_t);
static disassembly_action disassembly[] = {
    doSomethingOpcodeZero
,   doSomethingOpcodeOne
,   doSomethingOpcodeTwo
,   ...
};

Now you can use an array indexing instead of a switch:

void ClassA::SomeFunction() {
    uint8_t nibble1 = (opCode & 0xF000) >> 12;
    uint8_t nibble2 = (opCode & 0x0F00) >> 8;
    uint8_t nibble3 = (opCode & 0x00F0) >> 4;
    uint8_t nibble4 = (opCode & 0x000F);
    disassembly_action[something](nibble1, nibble2, nibble3, nibble4);
}

You can build a similar array to replace the other switch, perhaps using a different signature for your action functions.

like image 144
Sergey Kalinichenko Avatar answered Aug 24 '26 22:08

Sergey Kalinichenko



Donate For Us

If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!