bad code dept

Phil_aka_Pip

I found this just now:

int hexToChar(char c)
{
    char hexToCharResult = '\0';
 
    if(c=='0')
        hexToCharResult = 0;
    else if(c=='1')
        hexToCharResult = 1;
    else if(c=='2')
        hexToCharResult = 2;
    else if(c=='3')
        hexToCharResult = 3;
    else if(c=='4')
        hexToCharResult = 4;
    else if(c=='5')
        hexToCharResult = 5;
    else if(c=='6')
        hexToCharResult = 6;
    else if(c=='7')
        hexToCharResult = 7;
    else if(c=='8')
        hexToCharResult = 8;
    else if(c=='9')
        hexToCharResult = 9;
    else if(c=='A')
        hexToCharResult = 10;
    else if(c=='B')
        hexToCharResult = 11;
    else if(c=='C')
        hexToCharResult = 12;
    else if(c=='D')
        hexToCharResult = 13;
    else if(c=='E')
        hexToCharResult = 14;
    else if(c=='F')
        hexToCharResult = 15;
 
    return hexToCharResult;
}

frankus

the crazy - hasn't he ever heard of case statements

Phil_aka_Pip

apart from all the other obvious faults, what it actually does is the opposite of its name

frankus

that was my funny ship.

Phil_aka_Pip

frankus said:

that was my funny ship.

I thought you were commenting on the elseifs

mart

Beautiful. There is so much wrong with that code. He must have spent hours putting all that wrongness into what would originally have just been a line or two!

Phil_aka_Pip

mart said:

Beautiful. There is so much wrong with that code. He must have spent hours putting all that wrongness into what would originally have just been a line or two!

or even just a library routine call

equalsql

Yea.. should have used a 'select case'  method instead   

jellyroll

This is obviously an in-joke for computerists.

frankus

he's returning a local variable out of scope, thats the biggest sin. I've seen people sacked for that.


actually he's not .. my bad - not coded C for a long time

Phil_aka_Pip

If I had to do that by hand (no library routines available), I'd say

int HexCharToInt (char c)
{
  int result = 0;
  if ((c >= '0') && (c <= '9'))
  {
    result = c - '0'; // numeric
  }
  else
  {
    c &= 0xbf;  // to upper, if alpha
    if ((c >= 'A') && (c <= 'F'))
    {
      result = c - 'A' + 10;  // hexchar
    }
  }
  return(result);
}

other trickery might get you tighter code, but this is efficient enough, copes with upper and lower case, and is quite perspicuous about its purpose and method.

but in context, I think he's calling it per character in a string. so why not pass the string address into the library routine atol ?

frankus

can't you just subtract a number from ord?

imalone

Phil_aka_Pip said:

apart from all the other obvious faults, what it actually does is the opposite of its name

Though I'm not sure what the name actually should do when you analyse it closely either...
Strictly speaking I'm not sure there's a standard C function that does this (character, not string). There's a very quick cheat if you can guarantee ASCII, though you lose a bit of elegance if you properly generalise it and check inputs. Bizarre way to initialise the result all things considered.
(Edit, A-F aren't guaranteed contiguous... though it'd be an evil character set that broke that.)

Phil_aka_Pip

frankus said:

can't you just subtract a number from ord?

If I understand your question, the answer is no because the space you're decoding from is not contiguous. The number by which you nobble depends on the range of characters you're nobbling from.

jellyroll

Eh?

mart

jellyroll said:

Eh?

That's 65 in ascii.

Phil_aka_Pip

mart said:

jellyroll said:

Eh?

That's #41 in ascii.

fixed that for you

Hootsmon

I had a bad code but benylin and some aspirin cleared that Mother up

equalsql

hootsmon said:

I had a bad code but benylin and some aspirin cleared that Mother up

Sounds like you're still bunged up a bit  >:D<

Cacofonix

Phil_aka_Pip said:

I found this just now:

int hexToChar(char c)
{
    char hexToCharResult = '\0';
 
    if(c=='0')
        hexToCharResult = 0;
    else if(c=='1')
        hexToCharResult = 1;
    else if(c=='2')
        hexToCharResult = 2;
    else if(c=='3')
        hexToCharResult = 3;
    else if(c=='4')
        hexToCharResult = 4;
    else if(c=='5')
        hexToCharResult = 5;
    else if(c=='6')
        hexToCharResult = 6;
    else if(c=='7')
        hexToCharResult = 7;
    else if(c=='8')
        hexToCharResult = 8;
    else if(c=='9')
        hexToCharResult = 9;
    else if(c=='A')
        hexToCharResult = 10;
    else if(c=='B')
        hexToCharResult = 11;
    else if(c=='C')
        hexToCharResult = 12;
    else if(c=='D')
        hexToCharResult = 13;
    else if(c=='E')
        hexToCharResult = 14;
    else if(c=='F')
        hexToCharResult = 15;
 
    return hexToCharResult;
}

Ohohohoho.  What a plonker.



:-??

Phil_aka_Pip

@Cacofonix that's a lot more polite than my reaction