Bool explanation?

[Raiju]

Recently I was assigned to create a function that would return true if the string consisted of uppercase letters. I created this function but however whenever I run it, it states that not all control paths return a value. Could someone explain why it does that?

bool isUppercase(string text)
{
for (int k = 0; k < text.size(); k++)
{
if (text[k] = toupper(text[k]) || text =="")
return true;
else
return false;
}
}

[Markus]

Looking at it, I don't understand why you would get that error as all code paths do return a value?

[donbock]

What if text.size is <= 0? The for loop won't execute at all, the next statement is the closing brace of the function.

Perhaps it is impossible for text.size to be <= 0. Apparently the static analysis tool isn't smart enough to realize that. On the other hand, perhaps you only think it is impossible.

However, you have bigger problems. This function only tests the first character in the string. My impression is that you want to test all characters in the string. Can't help you without more information. What should the function return for the following strings:
"HELLO"
"Hello"
"hello"
"1234"

[Raiju]

I've rewrote it as
bool isUppercase(string text)
{
for (string::size_type k = 0; k < text.size(); k++)
{
if(isalpha(text[k]))
{
if (islower(text[k]))
return false;
}
}
return true;
}

which doesn't seem to have any problems. Thanks for the help.

[whodgson]

Yes because you used toupper (convert to upper case) instead of isupper (is already upper)

[weaknessforcats]

bool isUppercase(string text)
{
for (int k = 0; k < text.size(); k++)
{
if (text[k] = toupper(text[k]) || text =="")
return true;
else
return false;
}
}

Going back to your original question, the code above has an error that not all paths return a value. Note that the two return statements are part of the of the if statement.

You need a cosmtic return (one that is never reached but keeps the compiler happy):

bool isUppercase(string text)
{
for (int k = 0; k < text.size(); k++)
{
      if (text[k] = toupper(text[k]) || text =="")
      return true;
      else
      return false;
      }
 
      return false;  //cosmetic return
} 

or simply omit the else part of the if statement:

bool isUppercase(string text)
{
for (int k = 0; k < text.size(); k++)
{
      if (text[k] = toupper(text[k]) || text =="")
      return true;
 
      return false;  //cosmetic return
} 

[Ectara]

Also, in the if statement on line 5, make sure you use the == operator for making comparisons, rather than the = assignment operator. It would give unexpected results otherwise.

[Banfa]

@weaknessforcats
Actually this still has a path with no return statement, consider the case of isUppercase being passed an empty string, i.e. text.size() returning 0.

Assuming that the logic was correct (i.e. not calling toupper) then you need to return a value after the for loop.

[weaknessforcats]

Actually this still has a path with no return statement,

Oops. I missed that.