Bool explanation?

Collapse
X
 
  • Time
  • Show
Clear All
new posts
  • Raiju
    New Member
    • Oct 2009
    • 2

    Bool explanation?

    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(str ing text)
    {
    for (int k = 0; k < text.size(); k++)
    {
    if (text[k] = toupper(text[k]) || text =="")
    return true;
    else
    return false;
    }
    }
  • Markus
    Recognized Expert Expert
    • Jun 2007
    • 6092

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

    Comment

    • donbock
      Recognized Expert Top Contributor
      • Mar 2008
      • 2427

      #3
      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"

      Comment

      • Raiju
        New Member
        • Oct 2009
        • 2

        #4
        I've rewrote it as
        bool isUppercase(str ing text)
        {
        for (string::size_t ype 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.

        Comment

        • whodgson
          Contributor
          • Jan 2007
          • 542

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

          Comment

          • weaknessforcats
            Recognized Expert Expert
            • Mar 2007
            • 9214

            #6
            Originally posted by raiju
            bool isUppercase(str ing 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):

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

            Code:
            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
            }

            Comment

            • Ectara
              New Member
              • Nov 2009
              • 24

              #7
              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.

              Comment

              • Banfa
                Recognized Expert Expert
                • Feb 2006
                • 9067

                #8
                Originally posted by weaknessforcats
                Code:
                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
                }
                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.

                Comment

                • weaknessforcats
                  Recognized Expert Expert
                  • Mar 2007
                  • 9214

                  #9
                  Originally posted by Banfa
                  Actually this still has a path with no return statement,
                  Oops. I missed that.

                  Comment

                  Working...