Popa611
Popa611

Reputation: 130

C++ If else not working as intended? (Really weird bug)

I could use some help because I am really desperate now. Here, take a look at this piece of my code and then look at the output. What is causing this bug? How am I supposed to fix it?
Thanks for any kind of help!
The code:

while (1)
{
    while (1)
    {
     cout << "Choose your username: ";
     cin >> username;
     std::strcpy (username1, username.c_str());
     if (isdigit(username1[0]) || isdigit(username1[1]) || isdigit(username1[2]))
     {
      cout << "The first 3 characters HAVE to contain only letters!\n\a";
     }
          else if (username1[3] < 0)    // I don't actually know why, but this works as intended o.O
          {
              cout << "Your username HAS to contain atleast 3 letters!\n\a";
          }
               else if (username1[10] > 0)                      // Works - dunno why o.0
               {
                  cout << "Your username CAN ONLY contain maximum 10 characters!\n\a";
               }
                    else
                        break;
    }

The output:

Choose your username: ko
Your username HAS to contain atleast 3 letters!
Choose your username: k
Your username HAS to contain atleast 3 letters!
Choose your username: kokokokokoo    // Now that's > 10
Your username CAN ONLY contain maximum 10 characters!   // Now this is okay BUT...
Choose your username: ko
Your username CAN ONLY contain maximum 10 characters!  // Wrong error!
Choose your username: kok            // This should be accepted!
Your username CAN ONLY contain maximum 10 characters!  // Well, it is not... there should not be an error at all!

Pay attention to the comments I added in the output so you know what is wrong. Thank you for all answers!

Upvotes: 0

Views: 966

Answers (4)

Vlad from Moscow
Vlad from Moscow

Reputation: 310930

After you entered

kokokokokoo

and then ebtered

ko

character array username1 contains the following

[k] [o] ['\0'] [o] [k] [o] [k] [o] [k] [o] [o] ['\0']
 0   1     2    3   4   5   6   7   8   9   10   11

So this condition in the if statement

if (isdigit(username1[0]) || isdigit(username1[1]) || isdigit(username1[2]))

is false because neither of three characters [k] [o] ['\0'] is a digit.

This condition

else if (username1[3] < 0)

is also false because username1[3] is equal to 'o' which is greater than zero.

This condition

else if (username1[10] > 0)

is true because username1[10] is equal to 'o' that is greater than zero.

After you entered string literal

kok

username1 become to look as

[k] [o] [k] ['\0'] [k] [o] [k] [o] [k] [o] [o] ['\0']
 0   1   2    3     4   5   6   7   8   9   10   11

In fact there was nothing changed relative to the conditions. As username1[10] is equal to 'o' that is greater than zero then you get message

Your username CAN ONLY contain maximum 10 characters!

Also it is not clear why you copy object of type std::string username into the character array username1.

You could do all checks using username instead of username1 and after that you could copy username into username1 if it is required. For example

if ( username.size() < 3)
{
    cout << "Your username HAS to contain atleast 3 letters!\n\a";
}
else if ( username.size() > 10)
{
    cout << "Your username CAN ONLY contain maximum 10 characters!\n\a";
}
else if ( isdigit( username[0] ) || isdigit( username[1] ) || isdigit( username[2] ) )
{
    cout << "The first 3 characters HAVE to contain only letters!\n\a";
}
//...

Upvotes: 1

Karl Nicoll
Karl Nicoll

Reputation: 16419

The stuff in square brackets isn't doing what you think it's doing.

When you use the username1[10] code, what you're actually saying is "get me the character at position 11 in the string (remember, indexing starts at zero).

So the following line of code...

...
else if (username1[3] < 0)
...

Is literally saying "is the character at position 4 in the string less than zero?", and that's clearly nonsense (ascii characters can't be negative).

What you likely intended to say is "Is the length of username1 less than three characters?", in which case you should use the ::strlen method, like this:

else if (::strlen(username1) < 3)

As another answer mentions however, if you're using pure C++, there might be no need to use username1 at all, you could just call std::string::size() on the original username variable. Like this:

else if (username.size() < 3)

The reason that your code is working in unexpected ways is because you're trying to look for characters in the string that aren't there.

Imagine the following:

username1: [ k | o | k | o | \0 | 4 | f | 6 | 7 | a | 3  | 3 ]
  indexes:   0   1   2   3    4   5   6   7   9   9   10   11

The above is a basic image of some raw memory, as you can see, at location zero you have your string "koko", then a null terminator, and then lots of unrelated garbage that you haven't initialized. When you say username1[10], you're going past the end of the input string and into the garbage memory that has nothing to do with your string variable. So the if statement fails!

HOWEVER, because the characters in positions 5-11 are uninitialized, sometimes the character at position 10 could be zero, or it could be something else, or it could even cause your program to crash! This is what's known as undefined behaviour, and you should avoid it at all costs!

Upvotes: 2

user2357112
user2357112

Reputation: 280207

The parts where you don't know why they work don't work. You most likely don't need username1 at all; what you should really be doing is checking the input length with username.length():

if (username.length() < 3) {
    // "too short" error
else if (username.length() > 10) {
    // "too long" error

What you're doing is examining the (possibly indeterminate garbage) characters at indexes 3 and 10 of the string. The strcpy doesn't wipe index 10 when it copies something shorter into the string.

Upvotes: 0

Bryan
Bryan

Reputation: 12190

There's nothing setting username1[10] back to zero after you put the string "kokokokokoo" in it.

If you used strncpy instead of strcpy then (a) you would be safe from buffer overflows and (b) strncpy zero-pads the destination so your program would work as you hoped.

But really, as other comments have said, you should check the length of the string before you copy or use it.

Upvotes: 1

Related Questions