patterncModerate
Simple calculator
Viewed 0 times
simplecalculatorstackoverflow
Problem
I created my first program in C: a simple calculator. I want to know if it is possible to make this code more effective. Is it, for example, possible to change the calculator function so that I don't have to use switch, but the code knowing what to do because of the character that is entered?
#include
#include
char input[20];
char num[20];
int i;
int minus = 1;
float answer, num1, num2;
char operations;
void extractNumbers();
void calculate();
int main()
{
gets(input);
extractNumbers();
calculate();
printf("Answer: %f", num1);
return 0;
}
void extractNumbers(){
for(i = 0; i < 20; i++){
if(isdigit(input[i]) || input[i] == '.'){
strncat(num, &input[i], 1);
}
if((input[i] == '-' && i == 0) || (input[i] == '-' && !isdigit(input[i-1]))){
minus = -1;
continue;
}
if(input[i] == '+' || input[i] == '-' || input[i] == '*' || input[i] == '/' || input[i] == '^'){
num1 = atof(num) * minus;
minus = 1;
strcpy(num, "");
operations = input[i];
}
}
}
void calculate(){
num2 = atof(num) * minus;
switch(operations){
case '+':
num1 += num2;
break;
case '-':
num1 -= num2;
break;
case '*':
num1 *= num2;
break;
case '/':
num1 /= num2;
break;
}
}Solution
A few notes:
-
You see those global variables that you have at the top?
Those shouldn't be there. Those should be declared within functions. If another function required the value of one of those variables, pass it as a parameter. If you need to store a value from one function in another, return the value and store it in a variable.
-
You have the magic number 20 in your code.
-
Never ever ever EVER use
-
Declare your counter variable within your
-
You use some functions from the
-
Always declare what parameters your function takes in, even if nothing.
You might wonder why we have to do this. Imagine we have the function
In C, this is known as an identifier list and means that it "can take any number of parameters of unknown types". We can actually pass values to the function even though we don't mean to or intend to. If the caller calls the function giving it some argument, the behavior is undefined. The stack could become corrupted for example, because the called function expects a different layout when it gains control.
Using identifier lists in function parameters is depreciated. It is much better to do something like:
In C, this is known as a parameter type list and defines that the function takes zero arguments (and also communicates that when reading it) - like with all cases where the function is declared using a parameter type list, which is called a prototype. If the caller calls the function and gives it some argument, that is an error and the compiler spits out an appropriate error.
The second way of declaring a function has plenty of benefits. One of course is that amount and types of parameters are checked. Another difference is that because the compiler knows the parameter types, it can apply implicit conversions of the arguments to the type of the parameters. If no parameter type list is present, that can't be done, and arguments are converted to promoted types (that is called the default argument promotion).
-
Use
-
You don't have to return
C99 & C11 §5.1.2.2(3)
...reaching the
value of
-
-
You see those global variables that you have at the top?
char input[20];
char num[20];
int i;
int minus = 1;
float answer, num1, num2;
char operations;Those shouldn't be there. Those should be declared within functions. If another function required the value of one of those variables, pass it as a parameter. If you need to store a value from one function in another, return the value and store it in a variable.
-
You have the magic number 20 in your code.
-
Never ever ever EVER use
gets(). It is a dangerous function that is not to be used. Use fgets() instead, or gets_s() from C11.-
Declare your counter variable within your
for loops.for(int i = 0; i < 20; ++i)-
You use some functions from the
ctype.h library, but do not #include it.-
Always declare what parameters your function takes in, even if nothing.
int main(void)You might wonder why we have to do this. Imagine we have the function
foo() declared as such:int foo()In C, this is known as an identifier list and means that it "can take any number of parameters of unknown types". We can actually pass values to the function even though we don't mean to or intend to. If the caller calls the function giving it some argument, the behavior is undefined. The stack could become corrupted for example, because the called function expects a different layout when it gains control.
Using identifier lists in function parameters is depreciated. It is much better to do something like:
int foo(void)In C, this is known as a parameter type list and defines that the function takes zero arguments (and also communicates that when reading it) - like with all cases where the function is declared using a parameter type list, which is called a prototype. If the caller calls the function and gives it some argument, that is an error and the compiler spits out an appropriate error.
The second way of declaring a function has plenty of benefits. One of course is that amount and types of parameters are checked. Another difference is that because the compiler knows the parameter types, it can apply implicit conversions of the arguments to the type of the parameters. If no parameter type list is present, that can't be done, and arguments are converted to promoted types (that is called the default argument promotion).
char will become int, for example, while float will become double.-
Use
%g when printing float values. It also allows you to print double if you decide to "upgrade" the type later.-
You don't have to return
0 at the end of main(), just like you wouldn't bother putting return; at the end of a void-returning function. The C standard knows how frequently this is used, and lets you not bother.C99 & C11 §5.1.2.2(3)
...reaching the
} that terminates the main() function returns avalue of
0.-
minus should be declared as a int8_t from `, since you only use the values -1 and 1`.Code Snippets
char input[20];
char num[20];
int i;
int minus = 1;
float answer, num1, num2;
char operations;for(int i = 0; i < 20; ++i)int main(void)int foo(void)Context
StackExchange Code Review Q#63801, answer score: 13
Revisions (0)
No revisions yet.