13

So I have this huge tree that is basically a big switch/case with string keys and different function calls on one common object depending on the key and one piece of metadata.

Every entry basically looks like this

} else if ( strcmp(key, "key_string") == 0) {
    ((class_name*)object)->do_something();
} else if ( ...

where do_something can have different invocations, so I can't just use function pointers. Also, some keys require object to be cast to a subclass.

Now, if I were to code this in a higher level language, I would use a dictionary of lambdas to simplify this.

It occurred to me that I could use macros to simplify this to something like

case_call("key_string", class_name, do_something());
case_call( /* ... */ )

where case_call would be a macro that would expand this code to the first code snippet.

However, I am very much on the fence whether that would be considered good style. I mean, it would reduce typing work and improve the DRYness of the code, but then it really seems to abuse the macro system somewhat.

Would you go down that road, or rather type out the whole thing? And what would be your reasoning for doing so?

Edit

Some clarification:

This code is used as a glue layer between a simplified scripting API which accesses several different aspects of a C++ API as simple key-value properties. The properties are implemented in different ways in C++ though: Some have getter/setter methods, some are set in a special struct. Scripting actions reference C++ objects casted to a common base class. However, some actions are only available on certain subclasses and have to be cast down.

Further down the road, I may change the actual C++ API, but for the moment, it has to be regarded as unchangeable. Also, this has to work on an embedded compiler, so boost or C++11 are (sadly) not available.

Improve this question
8
  • 5
    Why the votes to close - this is a very good, valid question. What am I missing here? Commented May 17, 2012 at 15:47
  • 3
    It's a nice question, but "This question is not a good fit to our Q&A format. We expect answers to generally involve facts, references, or specific expertise; this question will likely solicit opinion, debate, arguments, polling, or extended discussion." Commented May 17, 2012 at 15:48
  • 4
    In C++ 11 you can use a dictionary of lambdas to simplify this. Commented May 17, 2012 at 15:50
  • 2
    I would not shy from the macros, als I think boost may have what you need in the preprocessor lib, but I have not used that yet Commented May 17, 2012 at 15:53
  • 1
    Can you show a more concrete example of what you have right now? Commented May 17, 2012 at 15:56

2 Answers 2

Reset to default
6

I would suggest you slightly reverse the roles. You are saying that the object is already some class that knows how to handle a certain situation, so add a virtual void handle(const char * key) in your base class and let the object check in the implementation if it applies to it and do whatever is necessary.

This would not only eliminate the long if-else-if chain, but would also be more type safe and would give you more flexibility in handling those events.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

This doesn't eliminate the long if-else-if chain, it merely moves/rearranges it. However, this is still a really good idea.
@MooingDuck It would not eliminate the individual ifs, but it would eliminate the central, long list of cases that have to be handled. Each check would be in the class where the actual handling happens, therefore the places that have to be changed to add new functionality would be close together.
That is an interesting idea. It would require me to modify the implementation classes, which for a variety of reasons is not desirable right now, but this is certainly a good idea.
Actually, I could implement the decision tree in an independant function with several specializations depending on the subclass type. interesting thoughts...
6

That seems to me an appropriate use of macros. They are, after all, made for eliding syntactic repetition. However, when you have syntactic repetition, it’s not always the fault of the language—there are probably better design choices out there that would let you avoid this decision altogether.

The general wisdom is to use a table mapping keys to actions:

std::map<std::string, void(Class::*)()> table;

Then look up and invoke the action in one go:

object->*table[key]();

Or use find to check for failure:

const auto i = table.find(key);
if (i != table.end())
    object->*(i->second)();
else
    throw std::runtime_error(...);

But if as you say there is no common signature for the functions (i.e., you can’t use member function pointers) then what you actually should do depends on the particulars of your project, which I don’t know. It might be that a macro is the only way to elide the repetition you’re seeing, or it might be that there’s a better way of going about it.

Ask yourself: why do my functions take different arguments? Why am I using casts? If you’re dispatching on the type of an object, chances are you need to introduce a common interface.

Improve this answer

2 Comments

"where do_something can have different invocations, so I can't just use function pointers." <- that looks like not all of the functions are going to be void(Class::*)(). Some might require arguments. At least that is what I understood.
Actually, I have two distinct kinds of invocations, so this actually would work if I used two maps.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.