6
\$\begingroup\$

I created this code to make a basic calculator after viewing a few examples. I added the function to continue the calculations myself. I am wondering if there is a way to make this more efficient as I feel I've used some unnecessary code but I can't make it work without.

I also was looking for feedback on my codes readability and how to improve if it is of a poor quality.

#include <stdio.h>

int main ()
{
int cancel = 1;
char o, end  = 'Y';
float numone = 0.0, numtwo = 0.0;

         /* Explaining structure and receiving inputs */

         printf("Please enter the two numbers you wish to manipulate!\n");
         scanf("%f", &numone);

         do {
                 scanf("%f", &numtwo);
                 printf("Please enter the operation you wish to perform (+, -, *, /)\n");
                 scanf(" %c", &o);

                 /* Checking for correct input and manipulating numbers with regards to user input */

                 switch(o){

                         case '+':
                                 printf("%.1f + %.1f = %.1f\n", numone, numtwo, numone + numtwo);
                                 break;
                         case '-':
                                 printf("%.1f - %.1f = %.1f\n", numone, numtwo, numone - numtwo);
                                 break;
                         case '*':
                                 printf("%.1f * %.1f = %.1f\n", numone, numtwo, numone * numtwo);
                                 break;
                         case '/':
                                 printf("%.1f / %.1f = %.1f\n", numone, numtwo, numone / numtwo);
                                 break;
                         default:
                                 /* Error message for incorrect input */
                                 printf("Error! Incorrect input!\n");
                                 break;
                 }`enter code here`
                 /* Option to continue calculations */

                 printf("Would you like to perform a further calculation? (Y/N)\n");

                 /* Checking for answer and terminating if N, resuming if Y */

                 scanf(" %c%", &end);
                        if (end == 'N') {
                         return 0;
                        }
                        else {}
                 numtwo = 0;

                 /* Requesting new number to use as numtwo for calculations */

                 printf("Current number = %f please input a second number!\n", numone);
                 } while (end == 'Y');

         return 0;
}
\$\endgroup\$
  • \$\begingroup\$ You should really learn to format your code properly, for your own benefit and for the benefit of anyone else who might have to read it - it will make it much easier to understand and debug. \$\endgroup\$ – Paul R Nov 3 '14 at 8:38
  • \$\begingroup\$ How do I format it properly. That's what I was wondering, to me it's readable so I don't know how to make it easier for others? \$\endgroup\$ – hysteriaistic Nov 3 '14 at 11:30
  • 1
    \$\begingroup\$ It's mainly that your indentation is all over the place and this makes the code hard to read - make the indentation consistent and logical - use 4 spaces for indent - make sure that you increase the indent level for each block and keep it the same within the block. I've re-formatted the code in your question now so that you can see the difference. \$\endgroup\$ – Paul R Nov 3 '14 at 12:03
  • 1
    \$\begingroup\$ Also: enable compiler warnings and take notice of them (and fix them!). \$\endgroup\$ – Paul R Nov 3 '14 at 12:04
  • 1
    \$\begingroup\$ Assuming you're using gcc then you just need to specify -Wall when compiling, e.g. gcc -Wall foo.c -o foo. Try and get into the habit of always enabling warnings - it will save you a lot of grief in the long run. \$\endgroup\$ – Paul R Nov 3 '14 at 14:51
3
\$\begingroup\$

Minor stuff:

I'm getting several compiler warnings. Capture the returned variable from scanf() to check if the user actually entered a number.

main.cpp:17: warning: ignoring return value of 'int scanf(const char*, ...)', declared with attribute warn_unused_result [-Wunused-result]
         scanf("%f", &numtwo);
                             ^

Your if-else statement needs to be fixed.

else {
numtwo = 0;
}
\$\endgroup\$
2
\$\begingroup\$
  1. Unused Variable

    You have an unused variable cancel. Remove it.

    int cancel = 1;
    
  2. Variable Names

    Use variable names that shows the purpose it is doing for better readability of your code, like the char o. Use 'operator' instead.

  3. scanf() and printf()

    1. Check if scanf() is reading correct value as @EngieOP said.

      Increase precision of the result:

      case '+':
          printf("%.2f + %.2f = %.2f\n", numone, numtwo, numone + numtwo);
          break;
      

      Your scanf("c%", &end); is giving a warning:

      calculator.c:46:9: warning: spurious trailing ‘%’ in format [-Wformat=]

    2. Try to use getchar() before scanf() to avoid unwanted results while reading char using scanf(), or check the return value.

      printf("Would you like to perform a further calculation? (Y/N)\n");     
      
      /* Checking for answer and terminating if N, resuming if Y */  
      getchar();  
      scanf(" %c", &end);
      
  4. When exiting the program check for both lower case and upper case of 'y':

    while (end == 'Y' || end == 'y');
    

    That also goes for 'N'.

  5. There is an else block after

    if (end == 'N') {
        return 0;
    }
    

    You don't have to use an else block after each if. It is doing nothing. Remove it.

  6. It is better to use a single return statement in main(). If the user presses 'N' or 'n' use break to break away from the loop.

    if (end == 'N' || end =='n') {  
        break;  
    }
    
  7. There is no division by zero check in your code.

    case '/':    
        if(numtwo == 0){  
            printf("Can't divide by zero.\n");  
            break;  
        }
    
        printf("%.1f / %.1f = %.1f\n", numone, numtwo, numone / numtwo);  
        break;
    
  8. Remove the numtwo = 0 near the end of the while loop; you will read a new numtwo anyway.

Here is your final code:

#include <stdio.h>

int main ()
{

    char operator, end  = 'Y';
    float numone = 0.0, numtwo = 0.0;

    /* Explaining structure and receiving inputs */

    printf("Please enter the two numbers you wish to manipulate!\n");
    scanf("%f", &numone);

    do {
        scanf("%f", &numtwo);
        printf("Please enter the operation you wish to perform (+, -, *, /)\n");
        scanf(" %c", &operator);

        /* Checking for correct input and manipulating numbers with regards to user input */

        switch(operator){

            case '+':
                printf("%.2f + %.2f = %.2f\n", numone, numtwo, numone + numtwo);
                break;
            case '-':
                printf("%.2f - %.2f = %.2f\n", numone, numtwo, numone - numtwo);
                break;
            case '*':
                printf("%.2f * %.2f = %.2f\n", numone, numtwo, numone * numtwo);
                break;
            case '/':
                if(numtwo == 0){  
                    printf("Can't divide by zero.\n");  
                    break;  
                }           
                printf("%.2f / %.2f = %.2f\n", numone, numtwo, numone / numtwo);  
                break;  
            default:
                /* Error message for incorrect input */
                printf("Error! Incorrect input!\n");
                break;
        }
        /* Option to continue calculations */

        printf("Would you like to perform a further calculation? (Y/N)\n");

        /* Checking for answer and terminating if N, resuming if Y */
        getchar();
        scanf(" %c", &end);
        if (end == 'N' || end =='n') {
            break;
        }           

        /* Requesting new number to use as numtwo for calculations */

        printf("Current number = %.2f please input a second number!\n", numone);
    } while (end == 'Y' || end == 'y');

    return 0;
}

Try learning about infix, postfix, prefix notations and stacks. They are easy to learn and with those you can build a far better calculator.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.