c - My program topped working when I enter a number

Question

I used to compile in VSCode with mingw-64 in Windows 10.

Please rectify my errors.

[1]: https://i.stack.imgur.com/j5CKZ.jpg [

#include <stdio.h>

int main() {
    float rupees,result;
    int choice;
    printf("Enter 1 for USD
");
    printf("Enter 2 for EUR
");
    printf("Enter 3 for AUS
");
    printf("Enter 4 for UAE Dirham
");
    printf("Enter amount in Rupees
");
    scanf("%d",&rupees);
    printf("Enter your choice
");
    scanf("%d",choice);
    switch (choice)
    {
    case 1:
        result=rupees*73.04 ;
         break;
        printf("%f Amount equal to %f USD
",rupees,result);
    case 2:
        result=rupees*88.82 ;
        printf("%f Amount equal to %f EUR
",rupees,result);
         break;

    case 3:
        result=rupees*56.36 ;
        printf("%f Amount equal to %f AUS
",rupees,result);
         break;

    case 4:
        result=rupees*19.89 ;
        printf("%f Amount equal to %f Dirham
",rupees,result);
         break;

    default:
    printf("Enter correct choice");
        break;
    }
    return 0;
}

question from:https://stackoverflow.com/questions/65847517/my-program-topped-working-when-i-enter-a-number

Answer 1

Firstly, Let me try to review your thought process before getting onto the code.

1. You declared and read variables to store the value in rupees, the user choice and the conversion result.
2. You wrote a switch case to activate a particular block of code based on user choice and in a default case, you wrote a "printf" statement to convey to the user that the choice was invalid.

However, the program is tainted and below are the reasons why:

1. The Logic is invalid, as per your program, 1 rupee must be equivalent to 73.04 USD, 88.82 EUR,56.36 AUS and 19.89 Dirham accordingly which is a falsity. The vice versa is true! 1 USD=73.04 Rupees and so on. Quick Fix would be to replace "*" operator with "/" operator at every switch case.

2. You cannot actually read a fractional value for rupees using the above code as the placeholder for a float data-type in C is "%f" and not "%d".

3. In the line where you wanted to read the user choice, you did not use the Address-Of Operator(&) i.e you did not give the address location into which the value is to be read, you gave the identifier itself.

4. In the first switch case, you placed the break statement before the "printf" statement. Which would mean that you would break the control flow even before you would print something on your console.

5. In a default case, you asked the user to enter a valid choice but the problem is that the control terminates thereafter leaving no scope to re-enter the choice. A quick fix to this is to put the switch case within a loop or alternatively, just display an error message stating "Invalid Choice". (Note that such issues are called semantic errors where the structure of the program is valid as per the language rules but the overall logic and flow of the application is inconsistent).

6. Miscellaneously, there are some message-format errors that could be made better. Ex: You don't have to ask to enter his/her choice given that you prior mentioned "Enter 1 for USD.....2 for EUR.....". Also, you could probably replace Amount with Rupees while printing the end-result to make the output more elegant and clear.

7. Last but not the least, the flow of your code is not so appropriate. Initially you display the entire catalogue to the user to choose from and then you ask the user to enter the value in rupees(you could ask the user to enter the rupee value irrespective of his conversion scale). By slightly rearranging the code, the usability of your script increases.

Wanna look at Rectified Code?????? There you go!!!!

Rectified Code:

#include <stdio.h>
    
int main()
{
float rupees,result;
int choice;
printf("Enter amount in Rupees
");
scanf("%f",&rupees);
printf("Enter 1 for USD
");
printf("Enter 2 for EUR
");
printf("Enter 3 for AUS
");
printf("Enter 4 for UAE Dirham
");
scanf("%d",&choice);

switch (choice)
{
case 1:
result=rupees/73.04 ;
printf("%f Rupees is equivalent to %f USD
",rupees,result);
break;
        
case 2:
result=rupees/88.82 ;
printf("%f Rupees is equivalent to %f EUR
",rupees,result);
break;

case 3:
result=rupees/56.36 ;
printf("%f Rupees is equivalent to %f AUS
",rupees,result);
break;

case 4:
result=rupees/19.89 ;
printf("%f Rupees is equivalent to %f Dirham
",rupees,result);
break;

default:
printf("That was an Invalid Choice!");
break;
}
return 0;
}