I'm learning some basics of C (I'm actually doing Harvard's cs50x) and I wrote this code:
#include <cs50.h>
#include <stdio.h>
int main(void) {
while(0 == 0) {
printf("1. SUM\n2. SUBSTRACTION\n3. MULTIPLICATION\n4. DIVISION\n5. QUIT\n");
int a = get_int("Choose one option: ");
if (a == 1) {
int x = get_int("x: ");
int y = get_int("y: ");
int sum = x y;
int sub = x - y;
int division = x/y;
int mul = x*y;
printf("Result = %i\n\n", sum);
}
else if (a == 2) {
int x = get_int("x: ");
int y = get_int("y: ");
int sum = x y;
int sub = x - y;
int division = x/y;
int mul = x*y;
printf("Result = %i\n\n", sub);
}
else if (a == 3) {
int x = get_int("x: ");
int y = get_int("y: ");
int sum = x y;
int sub = x - y;
int division = x/y;
int mul = x*y;
printf("Result = %i\n\n", mul);
}
else if (a == 4) {
int x = get_int("x: ");
int y = get_int("y: ");
int sum = x y;
int sub = x - y;
int division = x/y;
int mul = x*y;
printf("Result = %i\n\n", division);
}
else if (a == 5) {
printf("Alright! See you soon!\n");
break;
}
else {
printf("Invalid Input\n");
break;
}
}
}
It works exactly as I want it to, but I feel like it could be written in a better way.
I really don't like all that repetition of int variables and I think that it could be optimized someway but don't know how.
CodePudding user response：
All of the operations except for the "QUIT" and "invalid" operations have the lines
int x = get_int("x: ");
int y = get_int("y: ");
in common. Therefore, it would make sense to move these two lines outside the if...else if chain, so that you only have to write them once. However, the cases of "QUIT" and "invalid" must be handled beforehand, so that the user is not prompted to enter these two operands if he wants to quit or the input is invalid.
Also, you are always performing all 4 arithmetic operations (addition, subtraction, multiplication an division), although it would only be necessary to calculate the one that the user selected.
Additionally, I doubt that these lines are correct:
else {
printf("Invalid Input\n");
break;
}
If the user enters invalid input, you probably want to reprompt the user for input by restarting the loop, instead of exiting the program.
Instead of using a long if...else if chain, you can use a switch statement.
The line
while (0==0){
is technically correct, however it is very uncommon to write it that way.
In order to create an infinite loop, one would normally either write while (1) or for (;;), the latter being the more traditional syntax.
Here is a program which incorporates the mentioned improvements:
#include <cs50.h>
#include <stdio.h>
#include <stdlib.h>
int main( void )
{
for (;;)
{
printf(
"1. SUM\n"
"2. SUBSTRACTION\n"
"3. MULTIPLICATION\n"
"4. DIVISION\n"
"5. QUIT\n"
);
int a = get_int( "Choose one option: " );
if ( a == 5 )
{
printf( "Alright! See you soon!\n" );
//break out of infinite loop
break;
}
//verify that input is between 1 and 4
if ( ! ( 1 <= a && a <= 4 ) )
{
printf( "Invalid Input\n\n" );
//restart loop so that user gets reprompted for input
continue;
}
int x = get_int( "x: " );
int y = get_int( "y: " );
switch ( a )
{
case 1:
printf( "Result = %i\n\n", x y );
break;
case 2:
printf( "Result = %i\n\n", x - y );
break;
case 3:
printf( "Result = %i\n\n", x * y );
break;
case 4:
printf( "Result = %i\n\n", x / y );
break;
default:
//these lines should never be reached
fprintf( stderr, "internal program error!" );
exit( EXIT_FAILURE );
}
}
}
CodePudding user response：
@AndreasWenzel's answer is already quite good, using conditional guards and control flow tools like continue and break.
As an alternative, you can assign to a result variable in the switch, and then do a single printf for output.
int main(void) {
while (1) {
printf("1. SUM\n2. SUBSTRACTION\n3. MULTIPLICATION\n4. DIVISION\n5. QUIT\n");
int a = get_int("Choose one option: ");
if (a == 5) {
printf("Alright! See you soon!\n");
return 0;
}
else if (a < 1 || a > 4) {
printf("Invalid Input\n");
continue;
}
int x = get_int("x: ");
int y = get_int("y: ");
int result;
switch (a) {
case 1:
result = x y;
break;
case 2:
result = x - y;
break;
case 3:
result = x * y;
break;
case 4:
result = x / y;
break;
}
printf("Result = %i\n", result);
}
return 0;
}
As an alternative, we could check for good input and the quit choice as before, but use an array of function pointers to implement this quite concisely.
int add(int a, int b) { return a b; }
int sub(int a, int b) { return a - b; }
int mul(int a, int b) { return a * b; }
int div(int a, int b) { return a / b; }
typedef int(*op)(int, int);
int main(void) {
op ops[] = { add, sub, mul, div };
while(0 == 0) {
printf("1. SUM\n2. SUBSTRACTION\n3. MULTIPLICATION\n4. DIVISION\n5. QUIT\n");
int a = get_int("Choose one option: ");
if (a == 5) {
printf("Alright! See you soon!\n");
return 0;
}
else if (a < 1 || a > 4) {
printf("Invalid Input\n");
continue;
}
int x = get_int("x: ");
int y = get_int("y: ");
int result = ops[a - 1](x, y);
printf("Result = %i\n", result);
}
return 0;
}
Going a step further, we can break out the selection of an operation to perform into a separate function with all of the required input error handling, and group function pointers and menu options into structs.
typedef int(*op)(int, int);
typedef struct {
op operation;
char *menu_option;
} option;
option get_option(char *prompt, char * out_of_range_msg, char *out_of_tries_msg,
option *ops, size_t n, int tries,
int add_quit_option, char *quit_option_text, char *quit_msg) {
while (tries--) {
for (size_t i = 0; i < n; i) {
printf("-: %s\n", i 1, ops[i].menu_option);
}
if (add_quit_option) {
printf("-: %s\n", n 1, quit_option_text);
}
int opt = get_int(prompt);
if (opt < 1 || opt > (add_quit_option ? n 1 : n)) {
printf("%s\n", out_of_range_msg);
continue;
}
if (add_quit_option && opt == n 1) {
printf("%s\n", quit_msg);
exit(EXIT_SUCCESS);
}
return ops[opt - 1];
}
printf(out_of_tries_msg);
exit(EXIT_FAILURE);
}
Now main becomes cleaner still, and we see how easy it can become to add an additional operation.
int main(void) {
option ops[] = {
{addop, "Sum"}, {subop, "Subtraction"},
{mulop, "Multiplication"}, {divop, "Division"},
{modop, "Modulo"}
};
while (1) {
option op = get_option("Choose One option: ", "Invalid input", "Too many mistakes!",
ops, 5, 10,
1, "Quit", "Alright! See you soon!");
int x = get_int("x: ");
int y = get_int("y: ");
printf("Result: %i\n", op.operation(x, y));
}
return 0;
}
CodePudding user response：
Calculating all results in every case is kind of useless. The variables containing the results are quite useless aswell because you could make the calculation in the printf function, like this:
printf("Result = %i\n\n", x y);
lastly you could change the if statements to a switch case which would make the code look a bit cleaner, performance wise the switch case part wouldn't have any impact, but the other changes will.
And like Weather Vane commented, you can just do while(1).
CodePudding user response：
Use a switch() for example:
switch is like if -> if(value==1 or 2 or 3...)
switch(value){
case 1:
printf("Helllo!");
break; //you can use it to terminate your case switch instruction
case 2:
int sum= a b;
printf("Sum: %d", sum);
break;
case default: // value 1 or 2 have a case, but 27 not and this case represents undeclared values
printf("Error / invalid input");
break;
}
CodePudding user response：
To advance your code, you can create functions of the operations and then utilize them either with if...else ladder, or switch...case statements. You can look through the following code and ask any questions in mind. Please note that I am not a professional C programmer, and this is how I modified your code. I believe there will be much better ways of modifying your program.
#include <stdio.h>
int main(void) {
int a;
float x;
float y;
float result;
float my_remainder;
// Let's create functions first
float my_sum(float x, float y) // function for addition
{
result = x y;
};
float my_sub(int x, int y) // function for subtraction
{
result = x - y;
};
float my_mul(int x, int y) // function for multiplication
{
result = x * y;
};
float my_div(int x, int y) // function for divison
{
result = x / y;
my_remainder = x % y;
};
while(1){
start:
printf("1. SUM\n2. SUBSTRACTION\n3. MULTIPLICATION\n4. DIVISION\n5. QUIT\n");
choose: // let's choose an option, but can choose only one of the mentioned ones
printf("choose one option: ");
scanf("%d", &a);
switch(a) // check if "a" is not equal to either one of the mentioned numbers, then go back to 'choose' to ask for the right one
{
case 1: goto letsgo;
case 2: goto letsgo;
case 3: goto letsgo;
case 4: goto letsgo;
case 5: goto letsgo;
default:
printf("Invalid Input. Please choose an option from the given ones:\n\n");
goto start;
};
letsgo:
if(a == 5) // firstly, need to check QUIT condition, if QUIT is selected then the while loop stops (the process ends)
{
printf("\nAlright! See you soon!\n");
//return 0;
break;
}
else // if QUIT is not selected, it means one of the operations should be done
{ // so let's get the numbers for corresponding calculation
printf("Please enter first number: ");
scanf("%f", &x);
printf("Please enter second number: ");
scanf("%f", &y);
if(a == 1) // addition
{
my_sum(x,y);
printf("Result = %.2f", result); // %.2f <-- "2" represents the number of digits after the decimal separator.
return 0; // e.g., 1.4 1.2 = 2.60, however, if it was %.4f, then: 1.4 1.2 = 2.6000
}
else if(a == 2) // subtraction
{
my_sub(x,y);
printf("Result %.2f", result);
return 0;
}
else if(a == 3) // multiplication
{
my_mul(x,y);
printf("Result %.2f", result);
return 0;
}
else if(a == 4) // division
{
my_div(x,y);
printf("Result %.2f", result);
if(my_remainder != 0) // check if remainder is not equal to 0, then print, otherwise, don't print
{
printf("\nRemainder: %.2f", my_remainder);
}
return 0;
}
}
}
}
CodePudding user response：
this is the full code version
#include <stdio.h>
#include <stdlib.h>
int main() {
int choose = 0, num1, num2, work;
float div;
while(choose!=5){
printf(" 1. SUM\n 2. SUBSTRACTION\n 3. MULTIPLICATION\n 4. DIVISION\n 5. QUIT\n");
printf("Choose an option... \n");
scanf("%d", &choose);
printf("\nInsert two numbers: \n");
scanf("%d", &num1);
scanf("%d", &num2);
switch(choose){
case 1:
work = num1 num2;
printf("Sum: %d", work);
break; //you can use it to terminate your case switch instruction
case 2:
work = num1 - num2;
printf("Sub: %d", work);
break;
case 3:
work = num1 * num2;
printf("Multiplication: %d", work);
break;
case 4:
div = num1 / num2;
printf("Division: %f", div);
break;
case 5:
choose = 5;
break;
default:
printf("Error / invalid input");
break;
}
}
}