Connecting Tech Pros Worldwide Forums | Help | Site Map

Bool explanation?

Newbie
 
Join Date: Oct 2009
Posts: 2
#1: 4 Weeks Ago
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's Avatar
Moderator
 
Join Date: Jun 2007
Location: York, England, with wolves.
Posts: 4,949
#2: 4 Weeks Ago

re: Bool explanation?


Looking at it, I don't understand why you would get that error as all code paths do return a value?
Expert
 
Join Date: Mar 2008
Location: Naperville, Illinois U.S.
Posts: 831
#3: 4 Weeks Ago

re: Bool explanation?


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"
Newbie
 
Join Date: Oct 2009
Posts: 2
#4: 4 Weeks Ago

re: Bool explanation?


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.
Familiar Sight
 
Join Date: Jan 2007
Posts: 191
#5: 4 Weeks Ago

re: Bool explanation?


Yes because you used toupper (convert to upper case) instead of isupper (is already upper)
Moderator
 
Join Date: Mar 2007
Location: North Bend Washington USA
Posts: 5,379
#6: 4 Weeks Ago

re: Bool explanation?


Quote:

Originally Posted by raiju

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):

Expand|Select|Wrap|Line Numbers
  1. bool isUppercase(string text)
  2. {
  3. for (int k = 0; k < text.size(); k++)
  4. {
  5.       if (text[k] = toupper(text[k]) || text =="")
  6.       return true;
  7.       else
  8.       return false;
  9.       }
  10.  
  11.       return false;  //cosmetic return
or simply omit the else part of the if statement:

Expand|Select|Wrap|Line Numbers
  1. bool isUppercase(string text)
  2. {
  3. for (int k = 0; k < text.size(); k++)
  4. {
  5.       if (text[k] = toupper(text[k]) || text =="")
  6.       return true;
  7.  
  8.       return false;  //cosmetic return
Newbie
 
Join Date: Nov 2009
Posts: 20
#7: 3 Weeks Ago

re: Bool explanation?


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's Avatar
AdministratorVoR
 
Join Date: Feb 2006
Location: South West UK
Posts: 6,195
#8: 3 Weeks Ago

re: Bool explanation?


Quote:

Originally Posted by weaknessforcats View Post

Expand|Select|Wrap|Line Numbers
  1. bool isUppercase(string text)
  2. {
  3. for (int k = 0; k < text.size(); k++)
  4. {
  5.       if (text[k] = toupper(text[k]) || text =="")
  6.       return true;
  7.  
  8.       return false;  //cosmetic return

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.
Moderator
 
Join Date: Mar 2007
Location: North Bend Washington USA
Posts: 5,379
#9: 3 Weeks Ago

re: Bool explanation?


Quote:

Originally Posted by Banfa

Actually this still has a path with no return statement,

Oops. I missed that.
Reply