-10

I'm a very experienced Java programmer with a background in C++, but I'm just getting started with C in one of my programming classes and it's driving me nuts. This is the first assignment. It's supposed to calculate the volume or surface area of a sphere. The problem is that "radius" is equal to zero, even though the user enters the value. "mode" works just fine, which seems odd. Here is the code:

#include <stdio.h>
#define PI 3.14

int main()
{
    float radius; 
    int mode;

    printf("\nPlease enter a non-negative radius of a sphere: ");
    scanf("%f", &radius);

    printf("Please enter 1 for volume, 2 for area: ");
    scanf("%d", &mode);

    if (radius = 0)
    {
        printf("\n\nPlease enter a positive radius, instead of %f.", radius);
    }
    else
    {
        float area = 4 * PI * radius * radius;
        float volume = (4.0f / 2.0f) * PI * radius * radius * radius;
        float result = 0;

        if(mode == 1)
        {
            printf("\n\nYou are computing volume."); 
            result = volume;
        }
        else
        {
            printf("\n\nYou are computing area.");                              
            result = area;
        }

        printf("\n\nThe result is %2f", result); 
    }

    fflush(stdin);
    getchar();

    return 0; 
}

Any idea why radius isn't being stored correctly? FYI - most of this code was pre-written. I'm just supposed to find the errors.

rphello101
  • 1,671
  • 5
  • 31
  • 59
  • 1
    That shouldn't even compile. – Mat Aug 28 '13 at 06:08
  • compiling with warnings set to highest value will tell you this. -Wall – Mitch Wheat Aug 28 '13 at 06:10
  • Use `gcc -Wall`. You can use `-g` if you want to debug your code using `gdb`. – 0xF1 Aug 28 '13 at 06:13
  • 7
    From where you people are getting this `fflush(stdin)` idea? What does even suggest that it would be a good idea? (It isn't, it's UB.) Also, don't use `scanf()` for getting user input. It just doesn't do what you think it does. You are looking for `fgets(buf, sizeof buf, stdin)` and then `float radius = strtof(buf, NULL);`. Furthermore, the volume of a sphere is `4/3 pi r^3`, and **not** `4/2 pi r^3`... **Elementary maths, people!** –  Aug 28 '13 at 06:14
  • @H2CO3 - most of what you said is way over my head, but that's why I put the comment in that this was pre-written code. I didn't choose scanf() or fflush(stdin). As for the 4/3, that is also correct and apparently one of the errors I'm supposed to find. – rphello101 Aug 28 '13 at 06:15
  • @rphello101 Of course those apply to whomever wrote the code - if it's not you, then those errors are obviously not your fault! I didn't mean to offend by any means. Still, please spend some more time on individual learning. Thanks! –  Aug 28 '13 at 06:16
  • 1
    @rphello101 Excuse me, but Mitch Wheat wasn't arrogant... (nor an ass) –  Aug 28 '13 at 06:22
  • @H2CO3 No offense taken! It was a constructive comment that was just beyond me. As for individual learning, turns out the problem was something I should have been able to catch anyway. I spent a few hours screwing around with this and thought it was my lack of C understanding. I just was looking for a new set of eyes and intelligent input. – rphello101 Aug 28 '13 at 06:23
  • @rphello101 (Just in case - people tend to get my comments wrong, because when so many things are incorrect in a piece of code, I usually address them individually, and the tremendous amount of corrections may sound as nitpicking to some people. Not to you, apparently, so thanks!) –  Aug 28 '13 at 06:25
  • 1
    Very experienced Java programmers will notice the bug within a fraction of a second of setting eyes on the code. – Jim Balter Aug 28 '13 at 06:37

3 Answers3

5

if (radius = 0) should be if (radius == 0)

John3136
  • 28,809
  • 4
  • 51
  • 69
  • 3
    and this is why you can always write `if(0 == radius)` to avoid such an error ;) When you miss one `=` code won't compile – emesx Aug 28 '13 at 06:09
  • @elmes but people do not seem to not want to do that. I agree with you 100%. – dcaswell Aug 28 '13 at 06:10
  • It takes some getting used to writing "Yoda Expressions", but they can prevent errors! – John3136 Aug 28 '13 at 06:11
  • 6
    @elmes But you don't write `if (0 == radius)` because it's unintuitive and it's backwards. Instead, you make an effort compiling your code with goddamn `-Wall`... –  Aug 28 '13 at 06:11
  • @elmes: why not to just `if (radius)` if comparing with zero? – m0nhawk Aug 28 '13 at 06:12
  • Oops... That's one I should have been able to catch. But sometimes it just takes another person to catch the error... Thanks for the quick, constructive response. I'll accept the answer as soon as I'm able. – rphello101 Aug 28 '13 at 06:14
  • @m0nhawk, unless the value is a true boolean like `hasFailed` or `userIsDead`, I find it's usually more readable to use proper comparisons. `radius` is not such a beast. In any case, it would be `if (!radius)` in this case which is even _less_ readable :-) – paxdiablo Aug 28 '13 at 06:16
  • @MitchWheat I must admit that I don't get it. (Sometimes I wish I was a native English speaker...) –  Aug 28 '13 at 06:19
  • 1
    @H2CO3: my meaning was "Exactly!" – Mitch Wheat Aug 28 '13 at 06:20
  • @MitchWheat That wasn't in the Urban Dictionary, maybe that's an excuse for me. So thanks! :) –  Aug 28 '13 at 06:21
  • 1
    @paxdiablo: Sound's reasonable. But here is it's even `float`, so the different kind of comparison should be used. – m0nhawk Aug 28 '13 at 06:24
4

This:

#define PI=3.14

Should be this:

#define PI 3.14

Also you have an assignment in if comparison: if (radius = 0), should be if (radius == 0). Moreover, you shouldn't compare float or double like this.

Community
  • 1
  • 1
m0nhawk
  • 22,980
  • 9
  • 45
  • 73
2

In C, = is an assignment operator, and == a relational one. When you write

if (radius = 0)

you are assigning zero to radius. In C, that assignment is an expression, and has a value. An assignment expression evaluates to the value assigned to the left hand side of expression. In this case, since you're assigning zero, the value of the entire assignment is zero, and 

if (radius = 0)

evaluates to zero, or false. As a "side effect" (that's the technical term) of the assignment, radius gets the value zero too. The side effect is, of course, unfortunate and unintended in this case.

So what you want, as others have pointed out, is the relational operator ==:

if (radius == 0)

This too is an expression with a value: it evaluates to true or false, which is what you want.

verbose
  • 7,827
  • 1
  • 25
  • 40