0

I have this code in C++ that executes arbitrary arithmetic based on the value of a variable:

int foo = {value};

float execute(float input_1, float input_2) {
    switch (foo) {
        case 1:
            return (input_1 + input_2);
        case 2:
            return (input_1 - input_2);
        case 3:
            return (input_1 * input_2);
        case 4:
            return (input_1 / input_2);
        default:
            return 0;
    }
}

This code snippet is a pretty big understatement - I wouldn't be posting here if it was this simple.

I'll have around 50 different cases for the foo value, which seems a bit clunky to just hard-code every value and then check the foo variable against every previous value one-by-one; especially since it's just mapping an incrementing value to an operator. Some sort of list or array that only executes the included code if the index is called, or something like that.

Is there any better way - that being cleaner code or speed?

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • A more object-oriented way would be to have an abstract base class with an `execute` function, and from that have `Add`, `Subtract`, `Multiply` and `Divide` classes. The a container of "operations" (typically a vector of pointer to the base class) which is then initialized with using the concrete class objects. Iterate over the container and call the `execute` function. Yes might be a lot of classes, but more scalable than a big `switch`. – Some programmer dude Jul 06 '23 at 07:29
  • 2
    Perhaps if you explained what you're actually doing, what problem you're trying to solve, we might be able to give you better answers and solutions? – Some programmer dude Jul 06 '23 at 07:29
  • 1
    I think switch is actually made for that kind of code. You could look into function-pointer lookup tables.... but I wouldn't for something as shown. Maybe for full-blown statemachines. – Yunnosch Jul 06 '23 at 07:29
  • 2
    Instead of hardcoding magic numbers you could've used a dedicated enumerator. – user7860670 Jul 06 '23 at 07:31
  • 1
    I support @Someprogrammerdude comment in the impression that helping needs more context. And I want to mention https://meta.stackexchange.com/questions/66377/what-is-the-xy-problem – Yunnosch Jul 06 '23 at 07:31
  • An array that executes code would be some variety of a vector of function pointers. `std::vector` for example. – john Jul 06 '23 at 07:31
  • Side note - there is no reason to use `float` here. `double` has more precision and is faster. – john Jul 06 '23 at 07:33
  • In this big switch, will all the cases be in the form of `return (input_1 operator input_2);`? – Augusto Vasques Jul 06 '23 at 07:37
  • "Some sort of [..] that only executes the included code if the index is called" thats what you already have, no? I mean you can use something else than a switch, but dont expect it to be shorter than adding `case` to the number and `return` to the expression to be executed. This is about as compact as it can get. – 463035818_is_not_an_ai Jul 06 '23 at 07:42
  • When you can categorize cases into some groups, you can divide the big switch-case to some smaller. (Code becomes to better a bit?) – fana Jul 06 '23 at 07:49
  • @john that's interesting, do you have any link to resource about double being faster? Anyway, I agree with others saying this switch case is not that bad and, probably, the most performant alternative you could get. – Federico Jul 06 '23 at 08:00
  • 1
    @Federico I think the true answer is 'it depends', but the advice to use double 'by default' is sound. Here's a [discussion about Intel hardware](https://stackoverflow.com/questions/3426165/is-using-double-faster-than-float) (rather old however). – john Jul 06 '23 at 08:09
  • Compiler might change that switch in array (or hash_map) of function pointer or similar... – Jarod42 Jul 06 '23 at 12:37

1 Answers1

4

Consider a hash table that maps the foo operation to a function pointer, member function, or lambda.

Classic C jump table:

#include <unordered_map>
#include <functional>
using namespace std;

typedef float (*OpFn)(float x, float y);

float add(float x, float y) { return x + y; }
float sub(float x, float y) { return x - y; }
float mult(float x, float y) { return x * y; }
float div(float x, float y) { return x / y; }
typedef float (*OperationFn)(float, float);

extern int foo;

float execute(float input_1, float input_2) {

    static std::unordered_map<int, OperationFn> optable = {
        {1, add},
        {2, sub},
        {3, mult},
        {4, div}
    };

    auto itor = optable.find(foo);
    if (itor != optable.end()) {
        return itor->second(input_1, input_2);
    }
    return 0;
}

C++ jump table using pointer to member functions:

class Foo {

    Foo() {
        optable = {
            {1, &Foo::add},
            {2, &Foo::sub},
            {3, &Foo::mult},
            {4, &Foo::div}
        };
    }


    int foo;

    float add(float x, float y) { return x + y; }
    float sub(float x, float y) { return x - y; }
    float mult(float x, float y) { return x * y; }
    float div(float x, float y) { return x / y; }

    typedef float (Foo::* FooOp)(float, float);
    std::unordered_map<int, FooOp> optable;


    float execute(float input1, float input2) {

        auto itor = optable.find(foo);
        if (itor != optable.end()) {
            auto& fn = itor->second;
            (this->*fn)(input1, input2);
        }
        return 0;
    };
};

Or map to lambda functions:

float execute(float input_1, float input_2) {


    static std::unordered_map<int, std::function<float(float, float)>> optable = {
        {1, [](float x, float y) {return x + y; }},
        {2, [](float x, float y) {return x - y; }},
        {3, [](float x, float y) {return x * y; }},
        {4, [](float x, float y) {return x / y; }},
    };

    auto itor = optable.find(foo);
    if (itor != optable.end()) {
        return itor->second(input_1, input_2);
    }
    return 0;
}

In all of your above scenarios you don't have to inline initialize the optable within the function. That can be initialized and stored elsewhere such as a global variable or as a member of a class.

selbie
  • 100,020
  • 15
  • 103
  • 173
  • I agree with your idea, but I'd use a different data structure. Since the index is an integer, I'd use a std::vector rather than a std::unordered_map. Then, the particular function could be called by simply using `optable[foo](input_1, input_2)` — or something along those lines — and there'd be no need for an iterator or a `find` call, and no need to manually specify the integer indices. – David Yockey Jul 06 '23 at 12:11
  • 1
    @DavidYockey - array/vector would be fine assuming that the integer range starts at 0 or 1 and doesn't have too many unassigned entries. – selbie Jul 06 '23 at 16:04
  • All of these will take inordinately more time to execute than the switch/case. It will probably take more code and be less readable too. – Artyer Jul 06 '23 at 17:04