Learning C and doing a basic triangle exercise.

Can anyone check my code if I done everything right and explain why I am getting this error: suggest parenthesis around ‘&&’ within ‘||’

#include <iostream>

int main()
{
    // enter 3 numbers for sides of triangle
    int a, b, c;
    std::cout << "Input three numbers: " << std::endl;
    std::cin >> a;
    std::cin >> b; 
    std::cin >> c;
  
    // check what kind of traingle it is and output result
    if(a == b  && b == c){
        std::cout << "You have an equalateral triangle." << std::endl;
    }else if(a == b && b !=c || a!=b && b==c || a==c && b!=c){
        std::cout << "You have an iso triangle." << std::endl;
    }else{
        std::cout << "You have an scalene triangle." << std::endl;
    } 
}

CodePudding user response：

The Error Messages you're receiving are a diagnostic that Clang can generate

If these are showing up as errors, you likely have -Werror enabled in your compiler options. You can remove that flag to stop promoting Warnings to Errors. Or, if you don't want it to issue warnings (promoted to errors) for what is (let's be honest) a pretty spurious diagnostic, you can add -Wno-logical-op-parentheses to the compiler options.

As to why this diagnostic is being generated in the first place: The Operator Precedence between && and || is not always obvious, especially to newer programmers, and Clang is suggesting you expressly specify the operator precendence to make sure that what you've written is what you intend. So it's asking you to rewrite these lines like this:

if(a == b && b == c) {
    std::cout << "You have an equalateral triangle." << std::endl;
} else if((a == b && b != c) || (a != b && b == c) || (a == c && b != c)) {
    std::cout << "You have an iso triangle." << std::endl;
} else {
    std::cout << "You have an scalene triangle." << std::endl;
}

The extra parenthesis make the warning go away.

As far as I can tell there isn't actually a logical error in your code (the boolean logic behaves as you expect it to), and indeed, if you were to send this code to a different compiler, they compile it without issue. So this is really just a particular quirk of Clang.

CodePudding user response：

The compiler is suggesting that you use parentheses to make your second if statement clearer/easier to parse.

In particular, you should group the && statements within the || statement using parentheses. That is, change this code

(a == b && b !=c || a != b && b == c || a == c && b != c)

to

((a == b && b != c) || (a != b && b == c) || (a == c && b != c))

It's worth noting that this is a "style error"; the first form is not strictly incorrect code (indeed I was able to compile and run it just fine in the absence of compiler flags), but it's good practice to write it the second way.

If you pass the -Wall flag to your compiler (or more specifically in this case, the -Wlogical-op-parentheses flag) this will show up as a warning. Passing -Werror changes these warnings to errors, which seems to be the case here. It's good practice to use both -Wall and -Werror, since the compiler will then point out potential clarity problems like this and force you to fix them before continuing the compilation.

CodePudding user response：

This is sure to divide opinion (please downvote if you are on the opposite side and perhaps even identify yourself - it might be interesting and nobody ought to take a downvote or an upvote personally) but I am of the ilk that you need to know the C grammar well before you attempt to push any code in production.

The fact remains that || has lower precedence than &&. And that should be as second nature to you as the precedence of the binary operators and * ought to be. Emphasising the obvious with parentheses introduces noise and makes your code harder to read. I'd advise you to switch off that specific warning, and press on.

Note in your particular case though the second conditional simplifies to

else if (a == b || a == c || b == c)

as you have already eliminated the equilateral possibility. Again, some software houses insist that if statements are all mutually excusive in the sense that they can be arbitrarily reordered without the code breaking. Yet more opinion I'm afraid.