HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Movement code from a game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
movementcodefromgame

Problem

Could you suggest ways of writing this code better?

bool success = true;

if(x == 0)
{
    if(do_up)
        run_do_up();
    else if(do_a)
        run_do_a();
    else if(do_n)
        run_do_n();
    else
        success = false;
}
else if(x == 1)
{
    if(do_n)
        run_do_n();
    else if(do_a)
        run_do_a();
    else
        success = false;
}
else//x == 2
{
    if(do_a)
        run_do_a();
    else if(do_n)
        run_do_n();
    else
        success = false;
}

Solution

I think I'd encode this into something on the order of a state machine--a table holding the conditions and actions associated with each, and a tiny bit of code to walk through those to carry out the tests:

#include 
#include 

#ifdef TEST
bool do_n, do_a, do_up;
bool success;
bool T = true;

void run_do_up() { std::cout  action;

     template 
     conditional_action(bool const &b, F &a) : condition(b), action(a) {}

     conditional_action() : condition(T), action([&]{success = false; }) {}
};

conditional_action actions[4][3] { 
    { { do_up, run_do_up }, { do_a, run_do_a }, { do_n, run_do_n } },
    { { do_n, run_do_n }, { do_a, run_do_a } },
    { { do_a, run_do_a }, { do_n, run_do_n } }
};

void exec(size_t x) { 
    for (auto const &a : actions[x])
        if (a.condition) {
            a.action();
            return;
        }
}

#ifdef TEST

int main() { 
    do_n = true;
    exec(0);

    do_a = true;
    exec(2);
}
#endif


Depending on viewpoint, you might want to add a { T, [&]{success = false; }} to the end of each row in the actions table. That's what the default ctor will fill those entries with anyway, but there's a fair argument to be made that since it was explicit in the original logic, it should remain explicit here as well.

There is probably at least a little argument to be made that this is kind of a borderline case. A case where there were more conditions and actions would favor this technique more strongly. Whether really it makes sense when you have only three conditions and actions may be open to a little more question.

Code Snippets

#include <iostream>
#include <functional>

#ifdef TEST
bool do_n, do_a, do_up;
bool success;
bool T = true;

void run_do_up() { std::cout << "did up\n"; }
void run_do_a() { std::cout << "did a\n"; }
void run_do_n() { std::cout << "did n\n"; }
#endif

struct conditional_action { 
     bool const &condition;
     std::function <void()> action;

     template <class F>
     conditional_action(bool const &b, F &a) : condition(b), action(a) {}

     conditional_action() : condition(T), action([&]{success = false; }) {}
};

conditional_action actions[4][3] { 
    { { do_up, run_do_up }, { do_a, run_do_a }, { do_n, run_do_n } },
    { { do_n, run_do_n }, { do_a, run_do_a } },
    { { do_a, run_do_a }, { do_n, run_do_n } }
};

void exec(size_t x) { 
    for (auto const &a : actions[x])
        if (a.condition) {
            a.action();
            return;
        }
}

#ifdef TEST

int main() { 
    do_n = true;
    exec(0);

    do_a = true;
    exec(2);
}
#endif

Context

StackExchange Code Review Q#43683, answer score: 11

Revisions (0)

No revisions yet.