CODEWITHSUNDEEP
cfunction
sara hamdy
sara hamdy

Reputation: 412

I can't understand why this code give me wrong results for multiplication ,division and subtraction?

I can't understand why this code give me wrong results for multiplication ,division and subtraction?? where it give me the result of any multiplication operation is zero and result of any division error finally results of subtraction always is 1 


#include <stdio.h>
long operations(char x,int num1 ,int num2 );

int main(void) {
    char x;
    int num1 , num2;
    long result;
    setbuf(stdout,NULL);
    result=operations(x,num1 ,num2 );

}
long operations(char x,int num1 ,int num2 )
{
    printf("please enter your operation sum (1) ,subtrcation(2),mul(3),div(4) :");
    scanf("%d",&x);

    if (x==(1||2||3||4))
    {
    printf("please enter the first and second numbers : \n ");
    scanf ("%d%d" ,&num1 ,&num2);
    }
    else
    {
        printf("error ") ;
    }

    switch (x)
    {
    case 1:
    {
        return  printf("the result  sum is  :%d ",(num1+num2)) ;

    }
    case 2:
    {
        return  printf("the result of subtraction  is :%d ",(num1-num2)) ;

    }
    case 3:
        {
            return  printf("the result multiblication is :%d ",(num1*num2)) ;

        }
    case 4:
        {
            return  printf("the result division  is :%d ",(num1/num2)) ;

        }




    }
}

Upvotes: 0

Views: 111

Answers (2)

Odysseus
Odysseus

Reputation: 1263

As already mentioned by @chux this if statement is always true because true means != 0. Therefore every number not equal to 0 will match the condition.

Another way for the already suggested solution by @Haltarys is another switch - which might improve readability:

switch(x)
{
case 1: // fallthrough
case 2: // fallthrough
case 3: // fallthrough
case 4:
    printf("please enter the first and second numbers : \n ");
    scanf ("%d%d" ,&num1 ,&num2);
    break;
default:
    printf("error ");
    // consider returning here instead of break because you have no return below for this case
    break;
}

Since these are the same cases as in your switch one might think of combining them.

Some other hints concerning your code:

  • consider what happens if your inital condition fails (x == 5 e.g.) - Then you will print the error message but lack of a default statement for your switch and therefore missing a return value. Your compiler should have complained about that.
  • Your division won't print the result you want, since you print %d which means the value gets casted and you won't have decimal numbers printed - instead use %.2f e.g. to get 2 digits after decimal point and cast num1 in order not to get an int as result: (double)num1 / num2
  • You are not returning the result of your mathematical operation but the return value of the called printf which is the number of printed characters.

Upvotes: 0

Haltarys
Haltarys

Reputation: 626

You cannot "refactor" your condition x==(1||2||3||4)

You need to explicit write x == 1 || x == 2 || x == 3 || x == 4 instead.

Your compiler understands "if x is equal to 1 OR if x is equal to 2, etc."
It does not understand "if x is equal to 1, 2, 3, or 4"

Any number different from zero is considered true, so 1||2||3||4 is the same as 1.

If you want, you could write x >= 1 && x <= 4. You only check the lower and upper bounds.

Also, don't forget to check that num2 is different from 0 in the case of the division. Otherwise, your program will crash.

Upvotes: 5

Related Questions