| |
C programming problem |
| |
Cauchy (21:58 25/8/2015)
Cauchy (22:46 25/8/2015)
helpful (02:50 26/8/2015)
VincceH (08:56 26/8/2015)
Phlamethrower (09:02 26/8/2015)
VincceH (09:20 26/8/2015)
arawnsley (09:51 26/8/2015)
VincceH (17:04 26/8/2015)
Cauchy (13:23 26/8/2015)
|
| |
|
John O'Meara |
Message #123700, posted by Cauchy at 21:58, 25/8/2015 |
Member
Posts: 43
|
The following is not working.
int main(void)
{
int number; short i = 0; char ch;
char str[80];
printf("enter a number: ");
do {
ch = getchar();
str[i] = ch;
i++;
} while(ch != '\n' && !isalpha(ch));
number = atoi(str);
if(!isalpha(number))
printf("integer is: %d", number);
}
If I enter a 2 or 3 digit number like 123 it works ok, i.e., the last statement prints out;
integer is: 123, but if I enter a 4 or 5 digit number like 1234, the last statement of the code is skipped as if 'number' was an alphabelical character. I don't understand what is happening with the last if in the code. I compile it using Acorn's C/C++ C compiler. Does any have any ideas. Thanks in advance. |
| |
 [ Log in to reply ] |
| |
|
John O'Meara |
Message #123701, posted by Cauchy at 22:46, 25/8/2015, in reply to message #123700 |
Member
Posts: 43
|
Sorry to bother ye. It works with the gcc compiler; it must be a bug in Acorn's C/C++ compiler. Thanks. |
| |
[ Log in to reply ] |
| |
|
Bryan Hogan |
Message #123702, posted by helpful at 02:50, 26/8/2015, in reply to message #123701 |
Member
Posts: 254
|
OK, I'm not a C programmer, but surely isalpha(number) makes no sense? It is only designed to say whether a single character (0-255) is an alphabetic one, so passing it a large number is going to have undefined results.
(cue regular C programmers to point out how I've misunderstood things!) |
| |
[ Log in to reply ] |
| |
|
VinceH |
Message #123703, posted by VincceH at 08:56, 26/8/2015, in reply to message #123702 |
 Lowering the tone since the dawn of time
Posts: 1600
|
No, you're right - that's what it is.
It'll return true if the value passed is equivalent to an alphabetical character.
The first use of it in that example program - the condition in the do...while loop is fine, because that's checking the character entered (albeit that checking at that point is flawed - see below).
The second use, though, is checking the number - which may or may not be equivalent to an alphabetic character. isalpha() takes an int as its argument, which means you can pass larger values - as is the case here - but when you do I think its behaviour is undefined, so it's not surprising that GCC and Norcroft exhibit different behaviour.
Bottom line, though: it's not a bug in Norcroft.
Addressing the OP now.
You're building a number based on keyboard input, but by checking that the character is not an alphabetical one, you're still allowing other characters like ',.[]# etc to be input. Also, that check being part of the loop is wrong.
I'd suggest a better solution would be to check that the input character is between 48 and 57 before appending it to the string. (Since you're converting the string to an int, decimals are obviously irrelevant, so no need to look for '.')
Therefore, something like:
int main(void)
{
int number, ix=0;
char ch, txinput[10];
printf("Enter a number: ");
do
{
ch=getchar();
if( (ch>=48) && (ch<=57)) txinput[ix++] = ch;
} while( (ch!='\n') && (ix<10) )
number=atoi(txinput);
printf("Integer is %d",number);
}
Now, we're checking that the input character is 0-9 before appending it to the string - so the isalpha(..) in the while() is gone.
Also, note the additional check I've put in place in the do..while loop: That the number of digits the user inputs doesn't exceed the number of bytes assigned to the string being built up.
What should happen only the characters 0-9 are added to the string; anything else is ignored - except a hit on return. The loop will end if return is pressed - or if the user has input 10 digits.
The string isn't checked before being passed to atoi() because of how it's been built up - butin real world code, you might want to add suitable checks at that point as well. You can never be too careful!
(Final note: I typed this version from memory of the original because I was too lazy to copy and paste it into my reply for editing - so things like variables may not be the same. It can probably be greatly improved, but I tried to keep it as close to what I think the original was to make it more obvious to the OP)
[Edited by VincceH at 09:58, 26/8/2015]
[Edited by VincceH at 10:00, 26/8/2015] |
| |
[ Log in to reply ] |
| |
|
Jeffrey Lee |
Message #123704, posted by Phlamethrower at 09:02, 26/8/2015, in reply to message #123703 |
 Hot Hot Hot Hot Hot Hot Hot Hot Hot Hot Hot Hot Hot stuff
Posts: 15100
|
I'd suggest a better solution would be to check that the input character is between 48 and 57 before appending it to the string. (Since you're converting the string to an int, decimals are obviously irrelevant, so no need to look for '.') A more 'correct' way of doing that would be to use the isdigit() function, so that it will cope with non-ASCII systems.
But since you're highly unlikely to run it on a non-ASCII system you can probably just go for the simple range check. |
| |
[ Log in to reply ] |
| |
|
VinceH |
Message #123705, posted by VincceH at 09:20, 26/8/2015, in reply to message #123704 |
Lowering the tone since the dawn of time
Posts: 1600
|
I didn't know there even was an isdigit() function - but it makes sense if there's an isalpha() one! |
| |
[ Log in to reply ] |
| |
|
Andrew Rawnsley |
Message #123706, posted by arawnsley at 09:51, 26/8/2015, in reply to message #123705 |
R-Comp chap
Posts: 600
|
The method of building the string also looks a bit "off" to me. Writing directly to a char array won't null-terminate the string, so isn't there a risk of corruption coming in that way?
The source string isn't even initialised NULL, so I'd imagine there could be any old rubbish in RAM. |
| |
[ Log in to reply ] |
| |
|
John O'Meara |
Message #123707, posted by Cauchy at 13:23, 26/8/2015, in reply to message #123703 |
Member
Posts: 43
|
Thanks for the reply, ye are correct the whole thing was written incorectly, there is no bug in Acorn's C/C++ compiler, it is just a mistake I made in what I was trying to do. Thanks very much. I only use C on an on and off basis and more off than on at that. Thanks again. |
| |
[ Log in to reply ] |
| |
|
VinceH |
Message #123708, posted by VincceH at 17:04, 26/8/2015, in reply to message #123706 |
Lowering the tone since the dawn of time
Posts: 1600
|
The method of building the string also looks a bit "off" to me. Yes, well spotted. After exiting the loop, writing a null to the index position (which was being updated after each digit was added) in the character array will cover both those.
(Sorry, quickly hit reply without looking at the variable names...) |
| |
[ Log in to reply ] |
| |
|
