If statement always executing even when the condition is false?

Question

when it gets to this if statement, no matter what 'option' is inputted its always setting if(option == "y" || "Y") condition to true?

bool UserInterface::readInConfimDeletion() const
{
    string option = "";
    cout << "Are you sure you want to delete these transactions? Y/N "; 
    cin >> option;

    if (option == "y" || "Y")
    {
        return true;
    }
    else if (option == "n" || "N")
    {
        return false;
    }
    else{
        readInConfimDeletion();
        }

}

possible duplicate of [How do I compare strings in Java?](http://stackoverflow.com/questions/513832/how-do-i-compare-strings-in-java) — user207421, Apr 01 '15 at 10:49
The bottom line is to not treat C++ `if` conditions as if you're speaking English. *If option equals 'Y' or 'y'* may be understandable when speaking, but C++ doesn't work that way. — PaulMcKenzie, Apr 01 '15 at 10:52

score 4 · Answer 1 · edited May 23 '17 at 12:22

4

You can't compare multiple conditions like this:

if (option == "y" || "Y")

The "Y" condition will evaluate to true always if evaluated.

you need to do this:

if (option == "y" || option== "Y")

It would be simpler IMO to uppercase the string and perform a single comparision, there are a number of options to do this: Convert a String In C++ To Upper Case

So a possible solution would be:

#include <boost/algorithm/string.hpp>
string upperStr = boost::to_upper_copy(option);

then you can do:

if (upperStr == "Y")

edited May 23 '17 at 12:22

Community

1
1

answered Apr 01 '15 at 10:43

EdChum

376,765
198
813
562

1

Sadly, `std::toupper` only works for `char`, and option is a `std::string`. Still the best thing to do, but [with a library or simple support function](http://stackoverflow.com/questions/735204/convert-a-string-in-c-to-upper-case), or operating on `option[0]`, or changing `option` to `char` (all slightly different implications/compromises). – Tony Delroy Apr 01 '15 at 11:05
Thanks for pointing that out, for some reason I thought this would work, shouldn't `std::toupper(option.c_str())` work though? – EdChum Apr 01 '15 at 11:13
1

Nope... `std::toupper` won't accept the `const char*` `.c_str()` returns... I wish it was that easy! You can use `std::toupper(*option.c_str())`, but that's equivalent to `std::toupper(option[0])` anyway. The link in my comment above lists some options for handling full strings.... – Tony Delroy Apr 01 '15 at 11:20

score 2 · Answer 2 · answered Apr 01 '15 at 10:43

2

Change it like this:

if (option == "y" || option ==  "Y")

and similarly

else if (option == "n" || option == "N")

You cannot compare multiple strings like the way you are doing.

answered Apr 01 '15 at 10:43

Rahul Tripathi

168,305
31
280
331

score 2 · Answer 3 · answered Apr 01 '15 at 10:43

You need to say option == "y" || option == "Y" etc..

FYI / if (option == "y" || "Y") is actually asking if option == "y" or... "Y" itself is true, and "Y" is a string literal that undergoes a Standard Conversion to const char* then - being used inside an if undergoes a further conversion to the bool true (because the pointer is not nullptr, it's deemed true).

If statement always executing even when the condition is false?

3 Answers3