10

I have some untypical problem. I make available to the user sendText() function. He can type e.g. 

sendText( "mytext{newline}text{up}" )

{text} is a special key that user is allowed to send. There are a lot of special keys available.

So my first step was to get the string between {} brackets and to create:

if( _specialKey == "newline" ) { // action for VK_RETURN } else if( _specialKey == "up" ) { // action for VK_UP } else .....

example:

if( specialKey == "n" ) { // enter click unsigned short key = VK_RETURN; inputs.push_back( keyDown( key ) ); inputs.push_back( keyUp( key ) ); return 2; } else if( specialKey == "n+" ) { // enter down inputs.push_back( keyDown( VK_RETURN ) ); return 2; } else if( specialKey == "n-" ) { // enter up inputs.push_back( keyUp( VK_RETURN ) ); return 2; } else if( specialKey == "t" ) { // tabulator click unsigned short key = VK_TAB; inputs.push_back( keyDown( key ) ); inputs.push_back( keyUp( key ) ); return 2; } else if( specialKey == "t+" ) { // tabulator down inputs.push_back( keyDown( VK_TAB ) ); return 2; } else if( specialKey == "t-" ) { // tabulator up inputs.push_back( keyUp( VK_TAB ) ); return 2; } else if( specialKey == "caps" ) { // caps lock click unsigned short key = VK_CAPITAL; inputs.push_back( keyDown( key ) ); inputs.push_back( keyUp( key ) ); return 2; } else if( specialKey == "caps+" ) { // caps lock down inputs.push_back( keyDown( VK_CAPITAL ) ); return 2; } else if( specialKey == "caps-" ) { // caps lock up inputs.push_back( keyUp( VK_CAPITAL ) ); return 2; } else if( specialKey == "ralt" ) { // right alt click unsigned short key = VK_RMENU; inputs.push_back( keyDown( key ) ); inputs.push_back( keyUp( key ) ); return 2; } else if( specialKey == "ralt+" ) { // right alt down inputs.push_back( keyDown( VK_RMENU ) ); return 2; } else if( specialKey == "ralt-" ) { // right alt up inputs.push_back( keyUp( VK_RMENU ) ); return 2; } else if( specialKey == "lalt" ) { // right alt click unsigned short key = VK_LMENU; inputs.push_back( keyDown( key ) ); inputs.push_back( keyUp( key ) ); return 2; } else if( specialKey == "lalt+" ) { // right alt down inputs.push_back( keyDown( VK_LMENU ) ); return 2; } else if( specialKey == "lalt-" ) { // right alt up inputs.push_back( keyUp( VK_LMENU ) ); return 2; } else if( specialKey == "rctrl" ) { // right alt click unsigned short key = VK_RCONTROL; inputs.push_back( keyDown( key ) ); inputs.push_back( keyUp( key ) ); return 2; } else if( specialKey == "rctrl+" ) { // right alt down inputs.push_back( keyDown( VK_RCONTROL ) ); return 2; } else if( specialKey == "rctrl-" ) { // right alt up inputs.push_back( keyUp( VK_RCONTROL ) ); return 2; } else if( specialKey == "lctrl" ) {

but the compiler said:

fatal error C1061: compiler limit : blocks nested too deeply

My first idea to solve it was to define a map that will store all special keys (as a string) that maps to some integer. Then I could do:

switch( map[key] ) { case 0: ... }

but I'm not sure if the compiler won't complain about it too. There's a lot of to change, so I don't want to change it for no results.

Or maybe do you have some other better ideas?

Thanks.

Improve this question
16
  • 4
    How many else if statements do you have? Did you try a map of string==>action eliminating the if/switch entirely? Commented Jul 16, 2012 at 16:00
  • 10
    C1061 is "Nesting of code blocks exceeds the limit of 128 nesting levels. Simplify nesting.". You have 128 nested blocks of code? Either you have some horrific code there, or you have macros adding lots of unnecessary blocks. Try refactoring your code by splitting it up into helper functions etc. Commented Jul 16, 2012 at 16:00
  • 2
    Try using an array of key names and function pointers that do the relevant actions, then use a loop to find the key (binary search? hash even?) and perform the action. Commented Jul 16, 2012 at 16:02
  • 4
    @tobi I think this illustrates your code quite well: i.imgur.com/9hKqK.jpg Now, go and try to come up with a smarter implementation. Commented Jul 16, 2012 at 16:05
  • 3
    There is this amazing new thing you can do with C++. It's called "Object Oriented Programming". You should try it some time. Commented Jul 16, 2012 at 16:10

3 Answers 3

Reset to default
10

You have more than 127 else if blocks. While this ought to compile and it's certainly a bug in Microsoft's C++ compiler, it's still a pretty strong smell that something is wrong with your code.

You're storing data in your control flow, the vast majority of these 128 blocks are redundant copy and pasted blocks. You shouldn't be doing that if it's at all possible not to. Separate your code and data, use control flow for special cases while refactoring all the common cases into a single hash map that deals with it as one type.

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

1 Comment

Both C++ 1998 and C++ 2011 standards require 256 levels of nested block (see the first item in Appendix B, Implementation Quantities [implimits]). C only requires support for 127 levels, though.
6

You can use a map like: map<string, int>. One element would be like pair<string,int>("newline", VK_RETURN>:

map<string, int> NameToKey; NameToKey.insert(make_pair("newline", VK_RETURN));

And use the same in switch case like: 

map<string,int>::const_iterator iter = NameToKey.find(_specialKey); switch(iter->second) { case VK_RETURN: // Handle as "newline" ; }
Improve this answer

3 Comments

yea, how could I don't think about it, thanks I like the solution more than pointers to functions. But are you sure that there's no limitation about number of cases?
C++ 1998 Appendix B Implementation Quantities says Case labels for a switch statement (excluding those for any nested switch statements) [16 384]. A mere 156 labels should not be a problem.
If that is the case, you can logically divide them. Like given a range of VK_LBUTTON (1) to VK_F24 (135), you can use one switch (preferably in some function), and another into different function and so on. But I don't see you would need to handle ALL keys, there would be most of them in default ~
2

Little bit OOP instead pointer to functions way. 

class ICommand { public: virtual void execute() const =0; virtual void ~ICommand(){}; }; class KeyUpCommand : public ICommand { DWORD key_; //not suare about win api here public: KeyUpCommand(key) : key_(key) {}; virtual void execute() const { keyUp(key_); }; }; class KeyDownCommand : public ICommand { DWORD key_; //not suare about win api here public: KeyDownCommand(key) : key_(key) {}; virtual void execute() const { keyDown(key_); }; }; int main() { std::map<std::string, ICommand *> commands; commands["t-"]=new KeyUpCommand(VK_TAB); //execute now std::map<std::string, ICommand *>::const_iterator iter = commands.find(_specialKey); iter->second->execute(); };
Improve this answer

6 Comments

I like the solution the most.
Modified the answer that had some "copy-pasted" bug ;)
It was ;). but const_iterator should be ok, imho. I'am not sure about keyUp/keyDown signature.
and you can't type "virtual void execute()=0 const;" it should be "virtual void execute() const = 0;"
In reality, const_iterator may not work. The execute may itself would be needed to be non-const to actually "execute" something
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.