Programming Forums
User Name Password Register
 

RSS Feed
FORUM INDEX | TODAY'S POSTS | UNANSWERED THREADS | ADVANCED SEARCH

Reply
Page 1 of 2 1 2 >
 
Thread Tools Display Modes
Old Nov 4th, 2006, 7:01 PM   #1
Sane
Banned
 
Sane's Avatar
 
Join Date: Apr 2005
Location: Waterloo, Ontario
Posts: 2,101
Rep Power: 6 Sane will become famous soon enough
Send a message via MSN to Sane
Reducing Repetitive Code
I'm trying to figure out a way to reduce the repetitiveness of this code. I see a couple options: load an array of all the cases from a simplified data file, or declare an array of all the cases.

I could change it to a switch, but I don't feel like having 50 break statements clogging up my source.

The code converts an integer that represents the event of an asynchronous key's state, to a string representing the event's signal in human-readable form. Only the relevant signals are being handled, nothing like mouse clicks, F keys, arrows, etc...

There is no coreleation between the number and the ASCII value for those characters, only to the extent of how it is defined by the Windows operating system. If only there was a function that could do this conversion for me. Any ideas?

c Syntax (Toggle Plain Text) 
  1. boolean getShift () {
  2.         return (GetAsyncKeyState (160) || GetAsyncKeyState (161));
  3. }
  4.  
  5. boolean getCase () {
  6.         return (GetKeyState (20) & 1 ? getShift () == false : getShift () == true);
  7. }
  8.  
  9. void keyToString (unsigned char key, char *result) {         
  10.         boolean isUpper = getCase ();
  11.  
  12.         result[0] = 0;
  13.  
  14.         if (key >= 'A' && key <= 'Z') {
  15.                 result[0] = key + (isUpper ? 0 : 32);
  16.                 result[1] = 0;
  17.         }
  18.         else if (key >= 96 && key <= 105) {
  19.                 result[0] = key-48;
  20.                 result[1] = 0;
  21.         }
  22.         else if (key == 8) {
  23.                 strcpy (result, "^backspace^");
  24.         }
  25.         else if (key == 9) {
  26.                 strcpy (result, "^tab^");
  27.         }
  28.         else if (key == 13) {
  29.                 strcpy (result, "\n");
  30.         }
  31.         else if (key == 17) {
  32.                 strcpy (result, "^ctrl^");
  33.         }
  34.         else if (key == 18) {
  35.                 strcpy (result, "^alt^");
  36.         }
  37.         else if (key == 32) {
  38.                 strcpy (result, " ");
  39.         }
  40.         else if (key == 46) {
  41.                 strcpy (result, "^delete^");
  42.         }
  43.         else if (key == 48) {
  44.                 strcpy (result, isUpper ? ")" : "0");
  45.         }
  46.         else if (key == 49) {
  47.                 strcpy (result, isUpper ? "!" : "1");
  48.         }
  49.         else if (key == 50) {
  50.                 strcpy (result, isUpper ? "@" : "2");
  51.         } 
  52.         else if (key == 51) {
  53.                 strcpy (result, isUpper ? "#" : "3");
  54.         }
  55.         else if (key == 52) {
  56.                 strcpy (result, isUpper ? "$" : "4");
  57.         }
  58.         else if (key == 53) {
  59.                 strcpy (result, isUpper ? "%" : "5");
  60.         }
  61.         else if (key == 54) {
  62.                 strcpy (result, isUpper ? "^" : "6");
  63.         }
  64.         else if (key == 55) {
  65.                 strcpy (result, isUpper ? "&" : "7");
  66.         }
  67.         else if (key == 56) {
  68.                 strcpy (result, isUpper ? "*" : "8");
  69.         }
  70.         else if (key == 57) {
  71.                 strcpy (result, isUpper ? "(" : "9");
  72.         }     
  73.         else if (key == 186) {
  74.                 strcpy (result, isUpper ? ":" : ";");
  75.         }
  76.         else if (key == 187) {
  77.                 strcpy (result, isUpper ? "+" : "=");
  78.         }
  79.         else if (key == 188) {
  80.                 strcpy (result, isUpper ? "<" : ",");
  81.         }
  82.         else if (key == 189) {
  83.                 strcpy (result, isUpper ? "_" : "-");
  84.         }
  85.         else if (key == 190) {
  86.                 strcpy (result, isUpper ? ">" : ".");
  87.         }
  88.         else if (key == 191) {
  89.                 strcpy (result, isUpper ? "?" : "/");
  90.         }
  91.         else if (key == 192) {
  92.                 strcpy (result, isUpper ? "~" : "`");
  93.         }
  94.         else if (key == 219) {
  95.                 strcpy (result, isUpper ? "{" : "[");
  96.         }
  97.         else if (key == 220) {
  98.                 strcpy (result, isUpper ? "|" : "\\");
  99.         }
  100.         else if (key == 221) {
  101.                 strcpy (result, isUpper ? "}" : "]");
  102.         }
  103.         else if (key == 222) {
  104.                 strcpy (result, isUpper ? "\"" : "'");
  105.         }
  106. }
Sane is offline   Reply With Quote
Old Nov 4th, 2006, 9:56 PM   #2
grumpy
Programming Guru
 
grumpy's Avatar
 
Join Date: Jun 2005
Location: Adelaide, South Australia
Posts: 1,261
Rep Power: 5 grumpy will become famous soon enough
I haven't tested this with a compiler, but this should be enough to get you started.

Basic notion is that you have some ranges that you are mapping, and finding a technique to map anything in that range.
(Toggle Plain Text) 
boolean getShift () {
        return (GetAsyncKeyState (160) || GetAsyncKeyState (161));
}

boolean getCase () {
        return (GetKeyState (20) & 1 ? !getShift () : getShift ());
}

char ExtractChar(unsigned char key, int lower, int upper, const char *data)
{
     /* return mapped result, or zero if we can't do the mapping */

     return (key >= lower && key <= upper) ? data[key - lower]: '\0';
}

void keyToString (unsigned char key, char *result) {         
        boolean isUpper = getCase ();
     
        if (key >= 96 && key <= 105) {
                result[0] = key-48;
                result[1] = 0;
        }
        else if (key == 8) {
                strcpy (result, "^backspace^");
        }
        else if (key == 9) {
                strcpy (result, "^tab^");
        }
        else if (key == 13) {
                strcpy (result, "\n");
        }
        else if (key == 17) {
                strcpy (result, "^ctrl^");
        }
        else if (key == 18) {
                strcpy (result, "^alt^");
        }
        else if (key == 32) {
                strcpy (result, " ");
        }
        else if (key == 46) {
                strcpy (result, "^delete^");
        }
        else {
                result[1] = 0;
                if (!isUpper) {
                        if (key >= 'A' && key <= 'Z')
                               result[0] = key + 32;
                        else
                               result[0] = ExtractChar(key, 48, 57, "0123456789") ||
                                           ExtractChar(key, 186, 192, ";=,-./`") ||
                                           ExtractChar(key, 219, 222, "[\\]'");
                }
                else if (isUpper) {
                          if (key >= 'A' && key <= 'Z')
                                result[0] = key;
                          else
                                result[0] = ExtractChar(key, 48, 57, ")!@#$%^&*(") ||
                                            ExtractCha(key, 186, 192, ":+<_>?~") ||
                                            Extract(key, 219, 222, "{|}\"");
                }
      }
}
grumpy is offline   Reply With Quote
Old Nov 4th, 2006, 10:15 PM   #3
Sane
Banned
 
Sane's Avatar
 
Join Date: Apr 2005
Location: Waterloo, Ontario
Posts: 2,101
Rep Power: 6 Sane will become famous soon enough
Send a message via MSN to Sane
Ahh, yes. That's a good idea! I was trying to think of someway to use strings of data. This way looks perfect. I'll take your idea and come up with my own code. Thanks.
Sane is offline   Reply With Quote
Old Nov 7th, 2006, 12:47 PM   #4
2roll4life7
Programmer
 
2roll4life7's Avatar
 
Join Date: Aug 2005
Location: 0x0010 * 0x0091 + 0x0004
Posts: 65
Rep Power: 4 2roll4life7 is on a distinguished road
Quote:
Originally Posted by grumpy View Post
I haven't tested this with a compiler, but this should be enough to get you started.

Basic notion is that you have some ranges that you are mapping, and finding a technique to map anything in that range.
(Toggle Plain Text) 
boolean getShift () {
        return (GetAsyncKeyState (160) || GetAsyncKeyState (161));
}

boolean getCase () {
        return (GetKeyState (20) & 1 ? !getShift () : getShift ());
}

char ExtractChar(unsigned char key, int lower, int upper, const char *data)
{
     /* return mapped result, or zero if we can't do the mapping */

     return (key >= lower && key <= upper) ? data[key - lower]: '\0';
}

void keyToString (unsigned char key, char *result) {         
        boolean isUpper = getCase ();
     
        if (key >= 96 && key <= 105) {
                result[0] = key-48;
                result[1] = 0;
        }
        else if (key == 8) {
                strcpy (result, "^backspace^");
        }
        else if (key == 9) {
                strcpy (result, "^tab^");
        }
        else if (key == 13) {
                strcpy (result, "\n");
        }
        else if (key == 17) {
                strcpy (result, "^ctrl^");
        }
        else if (key == 18) {
                strcpy (result, "^alt^");
        }
        else if (key == 32) {
                strcpy (result, " ");
        }
        else if (key == 46) {
                strcpy (result, "^delete^");
        }
        else {
                result[1] = 0;
                if (!isUpper) {
                        if (key >= 'A' && key <= 'Z')
                               result[0] = key + 32;
                        else
                               result[0] = ExtractChar(key, 48, 57, "0123456789") ||
                                           ExtractChar(key, 186, 192, ";=,-./`") ||
                                           ExtractChar(key, 219, 222, "[\\]'");
                }
                else if (isUpper) {
                          if (key >= 'A' && key <= 'Z')
                                result[0] = key;
                          else
                                result[0] = ExtractChar(key, 48, 57, ")!@#$%^&*(") ||
                                            ExtractCha(key, 186, 192, ":+<_>?~") ||
                                            Extract(key, 219, 222, "{|}\"");
                }
      }
}
Nice idea, it'd probably look a little neater with a switch though, IMO.
__________________
(Toggle Plain Text) 
#if 0 /* in case someone actually tries to compile this */
- libpng version 1.2.8 (example.c)

<Jim_McNeat> Is there like a way to put a compiler in "Just trust me on that one" mode?
2roll4life7 is offline   Reply With Quote
Old Nov 9th, 2006, 4:43 AM   #5
grumpy
Programming Guru
 
grumpy's Avatar
 
Join Date: Jun 2005
Location: Adelaide, South Australia
Posts: 1,261
Rep Power: 5 grumpy will become famous soon enough
Quote:
Originally Posted by 2roll4life7 View Post
Nice idea, it'd probably look a little neater with a switch though, IMO.
Depends on your definition of "neat", I guess. Given that some of the conditions involve the value of "key" being in a range, that means a switch/case approach must list all the cases in that range. That can easily become unwieldy and difficult to maintain (eg it is easy to leave out one of the cases and that is hard to pick up without quite comprehensive testing).
grumpy is offline   Reply With Quote
Old Nov 9th, 2006, 1:06 PM   #6
Polyphemus_
Expert Programmer
 
Polyphemus_'s Avatar
 
Join Date: Aug 2005
Location: Rotterdam, the Netherlands
Posts: 942
Rep Power: 4 Polyphemus_ is on a distinguished road
Quote:
Originally Posted by grumpy View Post
Depends on your definition of "neat", I guess. Given that some of the conditions involve the value of "key" being in a range, that means a switch/case approach must list all the cases in that range. That can easily become unwieldy and difficult to maintain (eg it is easy to leave out one of the cases and that is hard to pick up without quite comprehensive testing).
You could create an if block for the ranges, and put the single keys in a switch. Not sure if that would look neater, though.
Polyphemus_ is offline   Reply With Quote
Old Nov 9th, 2006, 4:54 PM   #7
grumpy
Programming Guru
 
grumpy's Avatar
 
Join Date: Jun 2005
Location: Adelaide, South Australia
Posts: 1,261
Rep Power: 5 grumpy will become famous soon enough
2Roll4Life7 and Polyphemus_, if you want to play with getting it "neater"

1) Define what you mean by "neatness", and provide some measure by which you determine how neat some code is;

2) Try different approaches.

In keeping on topic with this thread, one measure might be related to the amount of repetitive code ......

In practice, I don't tend to like nesting switch/case blocks in if/else blocks (or vice versa) because it takes more time/effort to read and understand, which means it takes more time/effort to maintain.
grumpy is offline   Reply With Quote
Old Nov 9th, 2006, 5:21 PM   #8
Dizzutch
Professional Programmer
 
Dizzutch's Avatar
 
Join Date: Dec 2004
Location: Worcester, MA
Posts: 441
Rep Power: 4 Dizzutch is on a distinguished road
Send a message via ICQ to Dizzutch Send a message via AIM to Dizzutch Send a message via MSN to Dizzutch Send a message via Yahoo to Dizzutch
There is actually a term for the act of cleaning these kind of things up, it's called "Refactoring. As previously stated, a lot of it is just identifying what code could go into a function. Don't be afraid of having functions with long signatures. A common refactoring process is also creating large arrays with all the necessary information, sometimes even including function names and parameters.
example: if your program looks like this
(Toggle Plain Text) 
function bla
{
    if a then 1
    if b then 2
    if c then 3
    if d then 4
}
you could make an array
(Toggle Plain Text) 
{
( a, 1 )
( b, 2 )
( b, 3 )
( c, 4)
}
and replace the top code with something like this
(Toggle Plain Text) 
foreach $i in $array {
  if $i[0] then $i[1]
}
where $i[1] is actually an entire conditional and $i[2] is actually the name of a subroutine that can be parsed with some regex and used.

the string "subrouting:a:b:c"
could be parsed with
(Toggle Plain Text) 
/^subroutine:(.*):(.*):(.*)/
and called run as
(Toggle Plain Text) 
subroutine(a, b, c)
you can match the name of the subroutine to call the correct one in the event you need to call more than one.

now in some languages this is not as easy as in others (PERL is very good at things like this for example).
Excuse me for my rant on Refactoring, but it's a good skill to have, and I use it everyday for my job and on my personal projects. Eventually one can get good at it that the code written on the first try does not have to be refactored anymore, but don't be afraid to write it out the long way and look back afterwards.

Good Luck,

-Dizz
__________________
naked pictures of you | PFO F@H stats
Dizzutch is offline   Reply With Quote
Old Nov 10th, 2006, 2:53 AM   #9
Polyphemus_
Expert Programmer
 
Polyphemus_'s Avatar
 
Join Date: Aug 2005
Location: Rotterdam, the Netherlands
Posts: 942
Rep Power: 4 Polyphemus_ is on a distinguished road
Dizz! You're back!
Polyphemus_ is offline   Reply With Quote
Old Nov 11th, 2006, 12:38 AM   #10
2roll4life7
Programmer
 
2roll4life7's Avatar
 
Join Date: Aug 2005
Location: 0x0010 * 0x0091 + 0x0004
Posts: 65
Rep Power: 4 2roll4life7 is on a distinguished road
Well, what I meant was by putting all the numbers into a switch statement and then in the default you can have just a couple of if statments for the ranges. Or alternatively you could have listed all the numbers you wanted for a range with no break statements between them and have your code for that range after that. I'm not too crazy about either of those though. As Grumpy said, it depends on your definition of neat.

Those were just my observations from glancing at the code... If I were faced with this problem myself, I'd probably just do a lookup in a hard coded hash table (this doesn't reduce repetitive code, but I just think it's a better approach [assuming you only want a few selected key codes and not all]). Then again if we are stuck stictly with C, that would require writing your own hash table implementation or finding a decent one by somebody else. Perl could probably do this in 2 lines, lol, and even C++ would give you the option of the STL hash_set.

If you wanted to just map all the key codes with characters, you could do it with a large array of strings
eg:
(Toggle Plain Text) 
char *keycodes[] = {"VK_LBUTTON", "VK_RBUTTON ", "VK_CANCEL", "VK_MBUTTON ", "VK_XBUTTON1", "VK_XBUTTON2", NULL, "VK_BACK", "VK_TAB", "...etc..."};
Then instead of branching off to a function, you can just index into the array with the key and it'll give you the corresponding value. (you can even just put NULL if you don't want to map a specific one or two, but it must keep the proper indices.)

So both solutions I described don't actually reduce the code, but that doesn't mean they aren't good solutions. Infact, a hardcoded lookup table or an index into an array are both faster than branching into a function that performs calculations. Also, if you plan on maintaining this code, I'd go with whatever way you're more comfortable with.

If you'd like me to elaborate more, I won't mind, just ask.

* My example line from above was using the keycodes from this list: http://msdn.microsoft.com/library/de...alKeyCodes.asp
__________________
(Toggle Plain Text) 
#if 0 /* in case someone actually tries to compile this */
- libpng version 1.2.8 (example.c)

<Jim_McNeat> Is there like a way to put a compiler in "Just trust me on that one" mode?
2roll4life7 is offline   Reply With Quote
Reply
Page 1 of 2 1 2 >

Bookmarks

« Previous Thread in Forum | Next Thread in Forum »

Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes
Linear Mode Linear Mode

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts
BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Forum Jump

Similar Threads
Thread Thread Starter Forum Replies Last Post
EXECryptor software protection Jean5 C++ 35 Oct 10th, 2006 8:10 PM
Little help whoawhoayoyo Assembly 8 Apr 18th, 2006 8:10 PM
How to post a question nnxion C++ 10 Jun 3rd, 2005 12:53 PM
How to post a question nnxion C++ 0 Jun 3rd, 2005 9:55 AM
How to post a question nnxion C 0 Jun 3rd, 2005 9:55 AM


Contact Us - DaniWeb IT Discussion Community - Privacy Statement - Top


DaniWeb IT Discussion Community
All times are GMT -5. The time now is 10:41 AM.

Powered by vBulletin® Version 3.7.0, Copyright ©2000 - 2008, Jelsoft Enterprises Ltd.
Copyright ©2007 DaniWeb® LLC