9

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.

16
  • 4
    How many else if statements do you have? Did you try a map of string==>action eliminating the if/switch entirely?
    – Mark B
    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

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.

1
  • 2
    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. Commented Jul 16, 2012 at 16:13
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"
   ;
 }
3
  • 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?
    – tobi
    Commented Jul 16, 2012 at 16:09
  • 1
    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. Commented Jul 16, 2012 at 16:15
  • 1
    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 ~
    – Ajay
    Commented Jul 16, 2012 at 16:19
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();
};
6
  • I like the solution the most.
    – tobi
    Commented Jul 16, 2012 at 16:59
  • 1
    Modified the answer that had some "copy-pasted" bug ;)
    – Ajay
    Commented Jul 16, 2012 at 17:43
  • It was ;). but const_iterator should be ok, imho. I'am not sure about keyUp/keyDown signature. Commented Jul 16, 2012 at 17:53
  • 1
    and you can't type "virtual void execute()=0 const;" it should be "virtual void execute() const = 0;"
    – tobi
    Commented Jul 16, 2012 at 18:07
  • In reality, const_iterator may not work. The execute may itself would be needed to be non-const to actually "execute" something
    – Ajay
    Commented Jul 17, 2012 at 2:59

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.