expected ')' before string constant

[brucehoult]

You want something like:

Code: [Select]

if(key == '1' || key == '2' || ....


Why in %DEITY%'s name would you want to do it that way?

Because it will work fine and is easily derivable from the basic rules of the C language? Something which the original poster is clearly having trouble with.

Sure, it's a bit more typing than some other solutions, but if it takes 3 us on an AVR instead of 0.5 us, of what concern is that when the human is hitting a couple of buttons a second?

And it saves you from having to know that the digits have consecutive codes in ASCII, or that there is a library with an isdigit() function. Those can come later.

Incidentally, I tried compiling:

Code: [Select]

int isdig(char c){
    return
        (c == '0') || (c == '1') || (c == '2') || (c == '3') || (c == '4') ||
        (c == '5') || (c == '6') || (c == '7') || (c == '8') || (c == '9');
}

gcc -O for ARM gave:

Code: [Select]

00000000 <isdig>:
   0: 3830      subs r0, #48 ; 0x30
   2: b2c0      uxtb r0, r0
   4: 2809      cmp r0, #9
   6: bf8c      ite hi
   8: 2000      movhi r0, #0
   a: 2001      movls r0, #1
   c: 4770      bx lr

gcc -O for RISC-V gave:

Code: [Select]

0000000000000000 <isdig>:
   0: fd05051b          addiw a0,a0,-48
   4: 0ff57513          andi a0,a0,255
   8: 00a53513          sltiu a0,a0,10
   c: 8082                ret

gcc -O for AVR gave:

Code: [Select]

00000000 <isdig>:
   0: 90 ed        ldi r25, 0xD0 ; 208
   2: 98 0f        add r25, r24
   4: 81 e0        ldi r24, 0x01 ; 1
   6: 9a 30        cpi r25, 0x0A ; 10
   8: 08 f0        brcs .+2      ; 0xc <__zero_reg__+0xb>
   a: 80 e0        ldi r24, 0x00 ; 0
   c: 08 95        ret

Even gcc -O for x86_64 gave:

Code: [Select]

0000000000000000 <isdig>:
   0: 83 ef 30              sub    $0x30,%edi
   3: 40 80 ff 09          cmp    $0x9,%dil
   7: 0f 96 c0              setbe  %al
   a: 0f b6 c0              movzbl %al,%eax
   d: c3                    retq   

So, there's no need to actually know the values are dense in ASCII and optimise it yourself, because the compiler is perfectly capable of doing it for you, even at only -O1.

[brucehoult]

It illustrated the error and the correct way to do it. The fact that a function exists that simplifies this is immaterial. He clearly needs guidance on proper C coding.

No.  It illustrated the error and showed a technically correct way to do it  However, to become a well rounded and competent C coder, in addition to technical competence with the language, the O.P. needs experience in detecting and resolving 'code smells'.  If you don't point out the fuglyness of the proposed technically correct method, you are failing to help the O.P. develop a nose for bad code.

The repetition of testing the value against each member of the set of digits is undesirable in a C flow control statement that takes a boolean control parameter, as its possible to test if the value is within the limits of the set, or simply to hand off the task to the standard libraries, either of which significantly improve readability.   If however the O.P. was using a switch() ... case statement, as standard ANSI/ISO C/C++ doesn't support ranges in case labels, there's no alternative to explicitly testing each possible digit, though that may indicate that using switch() was a poor implementation choice.

A fine example of the code smell "premature optimisation". See my previous post. :-) :-)

[MarkS]

No.  It illustrated the error and showed a technically correct way to do it  However, to become a well rounded and competent C coder, in addition to technical competence with the language, the O.P. needs experience in detecting and resolving 'code smells'.  If you don't point out the fuglyness of the proposed technically correct method, you are failing to help the O.P. develop a nose for bad code.

No, the technically correct method is the one shown by joshtyler. This is basic C syntax. There are many ways to do things correctly in C, and that is why I love the language, but it is important to show the mistake and how to resolve it. You showed a different way, and in this instance the better way, but there will not always be a function to help him out and he'll need to know how to do proper testing of variables. To be pedantic, your way wouldn't help him if the '*', "#', 'A', 'B', 'C' or 'D' keys were pressed, so the method that Josh showed would have had to come into play at some point.

A fine example of the code smell "premature optimisation". See my previous post. :-) :-)

Very true.