-5

I am having problems with my convert String to upper case function. I have two functions right now reverse word and Upper case. reverse currently works and outputs information backwards but for some reason, uppercase wont. I have a menu which allows users to input a word then choose to either reverse or change it to uppercase

here is a snippet of code for the uppercase function.

string Upperword(string originalString){
    string localString;
    int len = originalString.length();

    for (int i = 0 ; i << len; i++)
        localString = toupper(originalString[i]);

    return localString;
}

anyone know why it wont output the word in uppercase format ?

user4581301
  • 33,082
  • 7
  • 33
  • 54
Gale23
  • 1
  • in c++ you have the std::string library, if you are dealing with a string using this then just call the function/method ".toupper()", if its a char array then its just toupper(). (http://www.cplusplus.com/reference/locale/toupper/) – SPlatten Mar 21 '19 at 16:07
  • Upvoted. This is as much of a fault caused by `std::string`s enormously bloated interface, rather than anything else. – Bathsheba Mar 21 '19 at 16:13

3 Answers3

3

As others have pointed out:

 i << len

Is incorrect. That expression will bit shift i to the left. Ultimately creating undefined behavior. I'm certain you meant this:

i < len

This line won't likely compile:

localString = toupper(originalString[i]);

You're trying to assign a character to a string. You want to be appending to the string.

You probably want something closer to this:

string Upperword(const string& originalString)
{
    string localString;
    int len = originalString.length();
    for (int i = 0 ; i < len; i++)
    {
        localString += toupper(originalString[i]);
    }
    return localString;
 }
selbie
  • 100,020
  • 15
  • 103
  • 173
3

Setting aside the typo i << len, you continuously reassign to the returned string, character by character; the extremely bloated list of functions provided by std::string obviates any compiler diagnostic.

A better way is

#include <algorithm>
std::string Upperword(std::string originalString){
    std::string localString = std::move(originalString);
    std::transform(
        localString.begin(),
        localString.end(),
        localString.begin(), 
        [](unsigned char c){return std::toupper(c);}
    );
    return localString;
}
Bathsheba
  • 231,907
  • 34
  • 361
  • 483
  • 3
    You should wrap `::toupper` in a lambda to make it more correct: `[](unsigned char c){ return std::toupper(c); }` see [cppreference](https://en.cppreference.com/w/cpp/string/byte/toupper) – Kevin Mar 21 '19 at 16:13
  • @Kevin: Well I'm not going to argue with that! You're hired. (And a frantic look through all my code...) – Bathsheba Mar 21 '19 at 16:15
0
  1. You need to set the size of your localString first. It is generated empty and thus setting its elements will make it crash. This can be done for example as localString.resize(originalString.length()); before you do anything.
  2. The condition should be i<len and preferably both len and i should be of type size_t
  3. Remember that toupper depends on the current locale, do not forget to set somewhere.
Alexey Godin
  • 307
  • 1
  • 6