Just a little exercise to make me more affluent with strings and error handling, any improvements welcome!
#include <stdio.h>
#include <string.h>
#include <ctype.h>
#include <stdbool.h>
#include <math.h>
#define MAXSIZE 1024
void retreiveStringInput(char *str, size_t buffersize)
{
for (;;) {
if (fgets(str, buffersize, stdin) != NULL) {
str[strcspn(str, "\n")] = 0;
return;
}
else
printf("Touble allocating memory, Please try again...\n");
}
}
bool strHasAllDigits(char *str)
{
size_t strsize = strlen(str);
if (str[0] != 0) {
for (size_t strIndex = 0; strIndex < strsize; ++strIndex) {
if (isdigit(str[strIndex]))
continue;
else
return false;
}
return true;
}
else {
printf("String is empty...\n");
return false;
}
}
int strToInt(char str[])
{
size_t len = strlen(str);
size_t power = len - 1;
int convertedString = 0;
for (size_t strIndex = 0; strIndex < len; ++strIndex, --power) {
convertedString += ((int)(str[strIndex] - '0') * pow(10,power));
}
return convertedString;
}
int main()
{
char number[MAXSIZE];
int intNumber = 0;
for (;;) {
printf("ENTER A WHOLE NUMBER: ");
retreiveStringInput(number, MAXSIZE);
if (strHasAllDigits(number))
intNumber = strToInt(number);
else {
printf("Number must be a whole number containing no characters, please try again...\n\n");
continue;
}
printf("Your number is: %d\n", intNumber);
break;
}
printf("Press any key to continue...");
getchar();
}
1 Answer 1
Nitpicks
void retreiveStringInput(char *str, size_t buffersize)
Misspelled.
void retrieveStringInput(char *str, size_t buffersize)
Not a critical mistake, since you used it consistently. Don't forget to change other uses when you fix it.
printf("Touble allocating memory, Please try again...\n");
This suggests that there is trouble allocating memory, but this code doesn't allocate memory. It reads from input.
It also has a typo in "Trouble".
Keeping it simple
size_t strsize = strlen(str); if (str[0] != 0) { for (size_t strIndex = 0; strIndex < strsize; ++strIndex) { if (isdigit(str[strIndex])) continue; else return false; } return true; } else { printf("String is empty...\n"); return false; }
Consider
if (*str == '0円') {
printf("String is empty...\n");
return false;
}
for (; *str != '0円'; ++str) {
if (!isdigit(*str)) {
return false;
}
}
return true;
I find it easier to use positive if
conditions with else
statements. So the if
is the positive case and the else
is the negative case. In this situation, we don't need an else
then, as we return in the if
.
There is no point in a continue
when there is no statement to skip. Just let it go. It will continue automatically. You don't need an else
case.
You only used the length as part of the condition check in the loop. But we don't need it. We can just check for the end of the string directly. We know that the end of string marker is '0円'
, so we can just check for that.
It's true that '0円'
is 0, but it is more readable to write it out in my opinion.
I changed str[0]
to *str
because I knew that I was going to be saying *str
in the for
loop anyway. Rather than use two different things that return the same value, I picked the one that worked in both cases. It would look kind of odd to repeatedly do str[0]
and act as if the values would be different. That's more understandable with *str
.
I don't like the single statement form of control structures. I always use the block form as being more robust against editing mistakes and a little easier to read.
Avoid unnecessary floating point operations
size_t power = len - 1; int convertedString = 0; for (size_t strIndex = 0; strIndex < len; ++strIndex, --power) { convertedString += ((int)(str[strIndex] - '0') * pow(10,power)); }
Consider
int convertedString = 0;
for (size_t strIndex = 0; strIndex < len; ++strIndex) {
convertedString *= 10;
convertedString += (int)(str[strIndex] - '0');
}
We don't need power
anymore.
We don't convert to and from a floating point type. No danger of precision losses. Everything is an integer type. We probably don't even need the explicit cast to int
. I haven't run the code, so I left it.
Simplify
if (strHasAllDigits(number)) intNumber = strToInt(number); else { printf("Number must be a whole number containing no characters, please try again...\n\n"); continue; } printf("Your number is: %d\n", intNumber); break;
Consider
if (strHasAllDigits(number)) {
int intNumber = strToInt(number);
printf("Your number is: %d\n", intNumber);
break;
}
printf("Number must be a whole number containing no characters, please try again...\n\n");
Now we don't have to declare intNumber
outside the loop. And we don't have to continue
to skip the break
.