Expert: Zlatko - 6/23/2010

Question
QUESTION: Hi Zlatko,

Once again, thank you very much for spending time with me on this program. Here is the revised code, sorry if the formatting is off. Anyhow, it does get the sum, but then it would print out the first number inputted by user then the actual sum. I thought it shouldn't do that now??

One other question: why you put 64 in the Char array size?  And lastly, it's the error checking, you said there was a problem with it and I am now attempting to make the error checking work so that it'll force user to enter a valid number from 0 to 99.


#include <stdio.h>
#include <string.h>
#include <ctype.h>
#define TRUE 1

int main()
{
  int i = 0, num_store = 0, number = 0, sum = 0;
  
  printf("ADD\n");
  printf("---\n");
  
  for(i=0; i <2; i++)
  {
     char input[64];  //An input array to hold the user input
     int j = 0, inputLen, inputIx; //inputLen = The length of the user input, inputIx = an index into the input array
     
     while (TRUE)
     {
        printf("Input a number: ");
        gets(input);
        
        if(isdigit(input[j]))
         break;
        
        printf("ERROR: invalid number entered. Please Re-Enter.");
}

// printf("The input is '%s'", input);
     inputLen = strlen(input);

// Loop to convert the characters into a number
     for (inputIx = 0; inputIx < inputLen; ++inputIx)
     {
// get the character
        char character = input[inputIx];

     /* subtract the value of the '0' character so that the
     character now contains the value meant by the input character.

     The character '1' is 49
     The character '2' is 50 and so on
     By subtracting the value of '0' (which is 48) from the character we get this:
     '0' - '0' = 48 - 48 = 0
     '1' - '0' = 49 - 48 = 1
     '2' - '0' = 50 - 48 = 2
     */
        character = character - '0';

     // Add in the character contribution to the number
        number = number * 10;
        number = number + character;

        if(i == 0)
         num_store = number;
     }
  }
  
  sum = number + num_store;
  printf("Sum = %d\n", sum);
  return 0;
}

ANSWER: Hello Mike
What happens if you input 123 for the first number and 456 for the second ? You get a sum of 123579. From the 123xxx part, it is obvious that the number, being multiplied by 10 for every digit, needs to be reset to 0 for the second input. Put number = 0 at the top of your "for i" loop.

I chose 64 for the input size for no particular reason. I like powers of 2. You need enough space to hold an integer and a null terminator byte and some extra space just in case the user enters a few too many characters. Actually this is only good if the user is co-operative. If the user wants to be mean, they can lean on the keyboard and overflow any buffer you set up. To do it properly, you need a special input function that reads the keyboard one character at a time and throws away any characters that would overflow the input buffer. That is probably more advanced than you need for your assignment.

Your error checking loop
while (TRUE)
       {
         printf("Input a number: ");
         gets(input);

         if(isdigit(input[j]))
         break;

         printf("ERROR: invalid number entered. Please Re-Enter.");
       }

has a problem. It checks only the first character. Notice that j is not incremented anywhere. What you need is another loop, with j as a counter, going from 0 to the length of input -1, and check each character. Rather than having yet another nested loop, consider using a function that returns 1 if all the characters are digits, and 0 if it finds a non-digit character. If you call your function isAllDigits, then your while loop can look like this:

while (TRUE)
       {
         printf("Input a number: ");
         gets(input);

         if(isAllDigits(input))
         break;

         printf("ERROR: invalid number entered. Please Re-Enter.");
       }

and you can remove the j variable from your main program.

Have you noticed that I am a big fan of breaking the program down into functions? It really makes the program easier to write and understand.

If you want to make sure the number is between 0 and 99, check that the length of the input is 2 or 1. Also remember that a length of 0 is no good so test that your program catches that.

Best regards
Zlatko

---------- FOLLOW-UP ----------

QUESTION: Hi Zlatko,

I agree that functions would probably have been a better way to go. Instead of putting all the tasks into one big long main function, you have little broken parts that do the independent single tasks.

Anyway, here's my code once more and I've added the error checking to block users from entering number outside of 0 to 99, but am still a little stuck on how to get the loop running for the isdigit error check. Thanks!!

#include <stdio.h>
#include <string.h>
#include <ctype.h>
#define TRUE 1

int main()
{
  int i = 0, numStore = 0, number = 0, sum = 0;
  
  printf("Add\n\n");
  
  for(i=0; i <2; i++)
  {
     number = 0;
     char input[16];  //An input array to hold the user input
     int j = 0, inputLen, inputIx; //inputLen = The length of the user input, inputIx = an index into the input array
     
     while (TRUE)
     {
        printf("Input a number: ");
        gets(input);
        
        if(isdigit(input[j]) && strlen(input)> 0 && strlen(input) < 3)
         break;
        
        printf("ERROR: invalid number entered. Please Re-Enter.\n\n");
}

// printf("The input is '%s'", input);
     inputLen = strlen(input);

// Loop to convert the characters into a number
     for (inputIx = 0; inputIx < inputLen; ++inputIx)
     {
        char character = input[inputIx];

    /* subtract the value of the '0' character so that the
    character now contains the value meant by the input character.

    The character '1' is 49
    The character '2' is 50 and so on
    By subtracting the value of '0' (which is 48) from the character we get this:
    '0' - '0' = 48 - 48 = 0
    '1' - '0' = 49 - 48 = 1
    '2' - '0' = 50 - 48 = 2
    */
        character = character - '0';

    // Add in the character contribution to the number
        number = number * 10;
        number = number + character;
        
        if(i == 0)
         numStore = number;
     }
  }
  
  sum = number + numStore;
  printf("\nANS: %d + %d = %d\n\n", numStore, number, sum);
  
  return 0;
}

ANSWER: Well Mike, Its never too late to start doing the right thing. If you agree that functions are better then make a function like this.

/*
Function isAllDigits.
Takes a string and returns 1 if all characters in the string are digits
and returns 0 on the first character
that is not a digit
*/
int isAllDigits(char* input)
{
/* TODO implement the function.
  You treat input just like an array
*/
}

Then replace the call to isdigit with the call to isAllDigits(input).


To do it without a new function will make your "while (TRUE)" loop too complicated. Part of the complication is caused by the break statement. It exits only the innermost loop and you need a new innermost loop to check all the characters of input.

If you want my advice, then that is it.

Best regards
Zlatko


---------- FOLLOW-UP ----------

QUESTION: I rewrote program using functions, I think it's much better looking now. So functions is the way to go all the time?


#include <stdio.h>
#include <string.h>
#include <ctype.h>
#define TRUE 1

void getNumber(char*);
int digitCheck(char*);

int main()
{
  int i = 0, number1 = 0, number2 = 0, add = 0;
  char input[16];
  
  printf("add two numbers\n\n");
  
  getNumber(input);
  number1 = digitCheck(input);

  getNumber(input);
  number2 = digitCheck(input);

  add = number1 + number2;

  printf("\nANS: %d + %d = %d\n\n", number1, number2, add);

  return 0;
}

void getNumber(char* input)
{
  //char input[16];  //An input array to hold the user input
     int j;
     
     while (TRUE)
     {
        printf("Input a number <0-99>: ");
        gets(input);

        j = digitCheck(input);
        
        if(j != -1)
         break;
        
        printf("ERROR: invalid number entered. Please Re-Enter.\n\n");
     }
}

int digitCheck(char* input)
{
   int i, inputLen, number = 0;
   
   inputLen = strlen(input);
   
   for (i = 0; i < inputLen; i++)
   {
     int digit = input[i] - '0';
        
     if ((digit<0 || digit>9) || (inputLen< 1 || inputLen>2))
        return -1;
     
     number *= 10;
     number += digit;
   }
   return number;
}

Answer
Hello Mike.

Good effort. Yes, functions are always the way to go. Even for a small assignment, your instructors will want to see that you can apply good programming principles which are crucial for large programs.

If you will be going further with programming, you should read "Code Complete" by Steve McConnell to learn what makes a good function.

Briefly, a function should do one thing and it should have a name that reflects its purpose. If you cannot think of a simple name for your function, it means that the function is doing too much. Your checkDigit is doing a bit more than its name says. It is also calculating the integer number from the string.  I realize that it is convenient to do so there because you already have a loop going through all the digits but this is not the best design because the two tasks, checking digits and calculating a number, are unrelated. Notice how you had to call digitCheck from your input loop, to check the digits, and then again from your main program to get the integer value. That's a little awkward and shows that perhaps the way you divided the program into functions is not optimal. However, your last effort was much better than the first. If I re-arrange your program only slightly, I can get rid of the awkwardness.

Also, a function should also have a detailed description of its inputs, outputs, and purpose at the top. You should write these things before writing your function. They will help you focus.

#include <stdio.h>
#include <string.h>
#include <ctype.h>
#define TRUE 1

int getNumber(void);
int digitCheck(char*);

int main()
{
   int number1 = 0, number2 = 0, add = 0;

   printf("add two numbers\n\n");

   number1 = getNumber();

   number2 = getNumber();

   add = number1 + number2;

   printf("\nANS: %d + %d = %d\n\n", number1, number2, add);

   return 0;
}

/*
function getNumber
gets a numeric string from the user and returns the integer
representation of the string
*/
int getNumber(void)
{
   char input[16];
   int  number = 0;
   int  inputLen;
   int  i;

   while (TRUE)
   {
       printf("Input a number <0-99>: ");
       gets(input);

       if (digitCheck(input)) break;

       printf("ERROR: invalid number entered. Please Re-Enter.\n\n");
   }

   inputLen = strlen(input);
   for (i = 0; i < inputLen; i++)
   {
       int digit = input[i] - '0';
       number *= 10;
       number += digit;
   }

   return number;
}

/*
function digitCheck
checks that input is all digits and checks that input length is 1 or 2
returns 1 if the input is good
returns 0 if the input is bad
*/
int digitCheck(char* input)
{
   int i, inputLen;

   inputLen = strlen(input);
   if (inputLen< 1 || inputLen>2) return 0;

   for (i = 0; i < inputLen; i++)
   {
       if (input[i]<'0' || input[i]>'9')
         return 0;
   }
   return 1;
}