Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It only takes a minute to sign up.
Sign up to join this community
Is it possible to make this code more efficient & elegant?
int HexToDecimal(char ch)
{
ch = tolower(ch);
if ('a' <= ch && ch <= 'f')
{
return ch - 'a' + 10;
}
else if ('0' <= ch && ch <= '9')
{
return ch - '0';
}
printf("Error\n");
return 0;
}
Update: Printing error could be ignored. By returning 0 I only mean to show that the input char is not a hex digit.
From memory (I haven't checked it) this should be the most efficient in terms of speed at the cost of a small increase in memory footprint.
Note that it does not repeat your printf("Error\n") smell.
return
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" // <nul> ... <si>
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" // <dle> ... <us>
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" // ' ' ... '/'
"\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x00\x00\x00\x00\x00\x00" // '0' ... '?'
"\x00\x0a\x0b\x0c\x0d\x0e\x0f\x00\x00\x00\x00\x00\x00\x00\x00\x00" // '@' 'A' ... 'O'
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" // 'P' ... 'Z' ... '_'
"\x00\x0a\x0b\x0c\x0d\x0e\x0f\x00\x00\x00\x00\x00\x00\x00\x00\x00" // '`' 'a' ... 'o'
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" // 'p' ... 'z' ... '~' <del>
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" /// Accented/Extended
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
[c];
Added
Apologies for not being clear. This solution is not "better" than the original, just different and provably faster. I would use the originally posted code in all cases except when maximum speed is critical and only then after careful deliberation.
I posted this for completeness, it is not elegant in any way, it is only more efficient ... almost brutally so.
\0xFF to indicate error would have been useful so that you can check for error.
\$\endgroup\$
Mar 15, 2012 at 16:44
c (which has a value of 0->255) as the index into the string. It returns a character from the string. The string is carefully constructed so that character 51 in the array returns the character \x03. Thus '3'(ie 51) when used as an index will return '\x03' which converts to the integer 3.
\$\endgroup\$
Mar 15, 2012 at 19:26
The first idea comming to mind is to check 0 <= ch <=9 before tolower(ch), because if it's a digit, then you don't need to make tolower.
Say, what does it means if the function returns 0? How to determine the case "it returns 0 because out of hex digits range" and "because ch is 0"?
As fogbit said, first check for digits, then tolower if necessary. By the way, unless you wrote your own tolower, it should probably be std::tolower.
Don't print anything on error. Either assert that the value is correct, return an impossible value (-1 is commonly used), or throw an exception. Printing that there was an error does not tell the rest of your program what happened.
Finally, do you really need to write this? Are you sure your numbers won't be in the form 0xABCD, which can be converted just fine with boost::lexical_cast or an std::stringstream wrapper?
Now for elegance (forgive me ... my C is a little rusty):
static char * Hex = "0123456789abcdef";
char * found = strchr(Hex, tolower(ch));
return found != null ? found - Hex: 0;
We search for the character in the string. If found then the value is the offset of the found character from the start of the string.