tags:

views:

107

answers:

4

I find myself writing code that looks like this a lot:

set<int> affected_items;
while (string code = GetKeyCodeFromSomewhere())
{
    if (code == "some constant" || code == "some other constant") {
        affected_items.insert(some_constant_id);
    } else if (code == "yet another constant" || code == "the constant I didn't mention yet") {
        affected_items.insert(some_other_constant_id);
    } // else if etc...
}
for (set<int>::iterator it = affected_items.begin(); it != affected_items.end(); it++)
{
    switch(*it)
    {
        case some_constant_id:
           RunSomeFunction(with, these, params);
        break;
        case some_other_constant_id:
           RunSomeOtherFunction(with, these, other, params);
        break;
        // etc...
    }
}

The reason I end up writing this code is that I need to only run the functions in the second loop once even if I've received multiple key codes that might cause them to run.

This just doesn't seem like the best way to do it. Is there a neater way?

+1  A: 

Since you don't seem to care about the actual values in the set you could replace it with setting bits in an int. You can also replace the linear time search logic with log time search logic. Here's the final code:

// Ahead of time you build a static map from your strings to bit values.
std::map< std::string, int > codesToValues;
codesToValues[ "some constant" ] = 1;
codesToValues[ "some other constant" ] = 1;
codesToValues[ "yet another constant" ] = 2;
codesToValues[ "the constant I didn't mention yet" ] = 2;

// When you want to do your work
int affected_items = 0;
while (string code = GetKeyCodeFromSomewhere())
    affected_items |= codesToValues[ code ];

if( affected_items & 1 )
    RunSomeFunction(with, these, params);
if( affected_items & 2 )
    RunSomeOtherFunction(with, these, other, params);
// etc...
Don Neufeld
+1  A: 

Its certainly not neater, but you could maintain a set of flags that say whether you've called that specific function or not. That way you avoid having to save things off in a set, you just have the flags.

Since there is (presumably from the way it is written), a fixed at compile time number of different if/else blocks, you can do this pretty easily with a bitset.

Greg Rogers
+2  A: 

One approach is to maintain a map from strings to booleans. The main logic can start with something like:

if(done[code])
    continue;
done[code] = true;

Then you can perform the appropriate action as soon as you identify the code.

Another approach is to store something executable (object, function pointer, whatever) into a sort of "to do list." For example:

while (string code = GetKeyCodeFromSomewhere())
{
    todo[code] = codefor[code];
}

Initialize codefor to contain the appropriate function pointer, or object subclassed from a common base class, for each code value. If the same code shows up more than once, the appropriate entry in todo will just get overwritten with the same value that it already had. At the end, iterate over todo and run all of its members.

Glomek
I like the todo-list idea.
Martin York
A: 

Obviously, it will depend on the specific circumstances, but it might be better to have the functions that you call keep track of whether they've already been run and exit early if required.

mbyrne215