Rate my simplified Calculator?

abcde597 · 10-22-2012, 08:16 PM

Code:

#include <stdio.h>
#include <stdlib.h>

char input [100];
float result;
float first;
float second;
char started;
char operation;

int main(int argc, char** argv) {
    result = 0;
    first = 0;
    second = 0;
    started = 'n';
    while (1) {
        if (started == 'n') {
            printf("Type all your equations in the correct form, examples: "
                    "2 + 2, 4 * 4, and so on, one space between the first number, the operation symbol, and the second number.\n");
            started = 'y';
            continue;
        }
        printf("Type 2 numbers like an equation, with a q in the middle to quit\n");
        fgets(input, sizeof(input), stdin);
        sscanf(input, "%f %c %f", &first, &operation, &second);
        switch (operation) {
            case '+':
                result = first + second;
                break;
            case '-':
                result = first - second;
                break;
            case '*':
                result = first * second;
                break;
            case '/':
                result = first / second;
                break;
            case 'q':
                break;
            case 'Q':
                break;
            default:
                break;
        }
        printf("Your answer is: %f\n", result);
    }
    return (EXIT_SUCCESS);
}

Any constructive criticism is appreciated.

millgates · 10-23-2012, 01:57 AM

Hi, that's a nice little program!
A few notes:
1)

Code:

float result;
float first;
float second;
char started;
char operation;

int main(int argc, char** argv) {
    result = 0;
    first = 0;
    second = 0;
    started = 'n';

Why don't you initialize the variables when you declare them?

Code:

float result = 0;
float first = 0;
float second = 0;
char started = 'n';
char operation;

Also, it does not really matter here, but I would make these variables local (maybe just my personal taste). I allways try to declare all variables in the smallest scope possible. In this case, result, first and second and operation could very well be declared even inside of the while loop.

2)

Code:

    started = 'n';
    while (1) {
        if (started == 'n') {
            printf("Type all your equations in the correct form, examples: "
                    "2 + 2, 4 * 4, and so on, one space between the first number, the operation symbol, and the second number.\n");
            started = 'y';
            continue;
        }
        ...
    }

I don't really understand this. If you want this to be done only in the first iteration, why don't you just put it out of the loop? That would also save you the need for the started variable:

Code:

   
    printf("Type all your equations in the correct form, examples: "
           "2 + 2, 4 * 4, and so on, one space between the first number"
           ", the operation symbol, and the second number.\n");

    while (1) {
        ...

3)

Code:

            case 'q':
                break;
            case 'Q':
                break;
            default:
                break;

No reason to put all those in your code when the result is the same. Just the default would suffice.
Also, this code will also not work as expected, since the break statement will take you out of the switch, but not out of the while loop.

Celyr · 10-23-2012, 03:20 AM

I suggest you also to have a look at this
http://en.wikipedia.org/wiki/Reverse_Polish_notation
so it may be easy to actually support expressions

abcde597 · 10-23-2012, 05:29 PM

1: Makes things simpler for me.

2: Wasn't thinking about it.
3: That's what was intended...
Just forgot an if for an exit. -.-