Why does my condition not work?

Question

I have the following code:

#include <iostream>

using namespace std;

int a, b, sqr;
const int P = 3.14; //Later for circles...
string s1; 

class MathsFunctions{
public:
virtual void square(int a, int b)=0;

};

class TriangleFunc: public MathsFunctions{
public:
    void square(int a, int b){
    sqr = (a * b)/2;
    cout << "Square of triangle is: "<< sqr << endl;
    }
};

class RectangleFunc: public MathsFunctions{
public:
    void square(int a, int b){
    sqr = a * b;
    cout << "Square of rectangle is:  "<< sqr << endl;
    }
};

void getNumbers(){
 cout << "Enter the first number:  "<<endl;
 cin >> a;
 cout << "Enter the second number:   "<< endl;
 cin >> b;
}
void chooseTheFigure(){
   cout << "Choose the figure (rectangle or triangle): "<< endl;
   cin >> s1;
}

int main(){

chooseTheFigure();
getNumbers();

if(s1 == "rectangle" || "Rectangle"){
RectangleFunc r;
MathsFunctions * m = &r;
m -> square(a,b);
};

if (s1 == "triangle" || "Triangle"){
    TriangleFunc t;
    MathsFunctions *m = &t;
    m -> square(a,b);
};

}

I created a program which is count the square of rectangle or triangle. There is a condition in main() but in the end program shows both results. How can I improve that?

Screenshot of output of the program:

Screenshot of output of the program

(s1 == "rectangle" || **s1 ==** "Rectangle") (s1 == "triangle" || **s1 ==** "Triangle") — Killzone Kid, Feb 11 '18 at 14:52
FWIW, Clang [with -Weverything](https://godbolt.org/g/Ve3yEk) gives some interesting stuff: converting 3.14 to 3 (without even enabling warnings), string literal to bool conversion (your problem), and lack of virtual destructor in your hierarchy. — chris, Feb 11 '18 at 14:58
In future, do not post [images of exceptions](http://idownvotedbecau.se/imageofanexception/). They are not useful for future readers or people trying to answer your question. Instead, post exceptions as properly formatted text. Thanks! — Micheal O'Dwyer, Feb 11 '18 at 15:05
@Nikita Why are you using global variables that are to be shared among classes? You need to learn how to use member variables! — Arnav Borborah, Feb 11 '18 at 15:15

WhiZTiM · Accepted Answer · 2018-02-11T15:03:07.050

This doesn't do what you think it does:

if(s1 == "rectangle" || "Rectangle"){
    RectangleFunc r;
    MathsFunctions * m = &r;
    m -> square(a,b);
};

The if-expression above is evaluated as:

if((s1 == "rectangle") || ("Rectangle"))
 // ^^^^^^^^^^^^^^^^^  or  ^^^^^^^^^^

Now, the second part there, "Rectangle" is a string-literal which implicitly converts to a valid pointer. And any pointer other than nullptr or some zero like integer evaluates to true - always.

You probably meant to write:

if((s1 == "rectangle") || (s1 == "Rectangle")){
    RectangleFunc r;
    MathsFunctions * m = &r;
    m -> square(a,b);
};

----------------------------------------

There are a few other nuances in your code, such

not having a vitual destructor in your base class and,

this:

const int P = 3.14; //Later for circles...

P will not hold the value you expect.

skaur · Answer 2 · 2018-02-12T15:01:37.450

1

1.As WhiZtim pointed out, Or operator needs correction

if(s1 == "rectangle" || "Rectangle") {...}

will always be true as "Rectangle" is not null.

You should use string compare functions for strings (see strcmpi())

Edit

Regarding string functions, check this out:

Case-insensitive string comparison in C++

edited Feb 12 '18 at 15:01

answered Feb 11 '18 at 14:57

skaur

168
2
11

1

Just a bit of an issue with your second point. There's no standard function like `strcmpi` in C++. Secondly if you meant [`std::strcmp`](http://en.cppreference.com/w/cpp/string/byte/strcmp), there's absolutely no god reason to do that for `std::string`. There is [`std::string::compare`](http://en.cppreference.com/w/cpp/string/basic_string/compare) if you need that style. And its often faster than `strcmp` for some corner cases. – WhiZTiM Feb 11 '18 at 15:11

score 1 · Answer 3 · answered Feb 11 '18 at 14:58

if(s1 == "rectangle" || "Rectangle"){

There are 2 conditions when this is true, the first is what you expect, the second is your bug because it's now what you meant to say in your code:

1) the input string s1, compared for equality to the string literal "rectangle" returns true, or

2) if the string-literal "Rectangle", by itself, is considered a true value. As this conversion is essentially a "null pointer check", and the string literal is never null, this case is always true.

What you need is to repeat the test:

if(s1 == "rectangle" || s1 == "Rectangle"){

Why does my condition not work?

3 Answers3

----------------------------------------