Returning null char array with rewriting Java's String.toUpperCase()

Question

I am originally a Java programmer and I have a deep love of the syntax, especially regarding the String object. With C++, I have tried to recreate the toUpperCase() method that Java has. The only problem is that it always returns a String object that has an empty/NULL char array.

String String::toUpperCase()
{
    char *a = new char[this->length + 1];
    memset(a, 0, this->capacity + 1);
    memcpy(a, this->characters, this->length);
    for (int i = 0; i < strlen(this->characters); i++)
    {
        toupper(a[i]);
    }
    return *new String(a);
}

You'd be wiser using something from here: http://stackoverflow.com/questions/735204/convert-a-string-in-c-to-upper-case?rq=1 and `std::string` in general. This function leaks memory every time it's called. Also, read some documentation for `toupper`. — chris, Mar 18 '14 at 13:30
Also: In Java and C# you use `new` a lot, in C++ you should avoid it when possible (no GC). — crashmstr, Mar 18 '14 at 13:35
As a general rule, do not try to recreate java class APIs in C++ classes (they are optimized for Java, so what you end up with tends to be a variation of the "I can write C code in any language" theme). To convert a C++ string to upper case, see [this answer](http://stackoverflow.com/a/735215/186997). — utnapistim, Mar 18 '14 at 13:37
On another note, stop abusing pointers (i.e. you should write `return String(a);` instead of 'return *new String(a);' and use a smart pointer on `a`, or better yet, a `std::vector`); Also, stop writing `this->`. It is unnecessary. Also consider using `std::fill` instead of `memset` and `std::copy` instead of `memcpy`. — utnapistim, Mar 18 '14 at 13:39

score 2 · Accepted Answer · answered Mar 18 '14 at 13:35

2

You have a few memory problems with your attempt, as well as a logical one. All you need to return a copy of a string with the characters being upper case is:

std::string str = "My Original string";
std::string myCopy(str);
std::locale loc;
std::transform(myCopy.begin(), myCopy.end(), myCopy.begin(), [&](char c)
{
    return std::toupper(c, loc);
});

answered Mar 18 '14 at 13:35

Zac Howland

15,777
1
26
42

or just `std::transform(myCopy.begin(), myCopy.end(), myCopy.begin(), std::toupper)` if you don't need locale sensitivity. – japreiss Mar 18 '14 at 13:36
1

@japreiss, That won't always work out so well, although the answer won't either if `char` is signed and there are negative values. You can check the link I commented for more discussion on this method. – chris Mar 18 '14 at 13:37
1

@japreiss I’d argue that not using a locale is always broken when operating on text – how can you exclude the need for using locales? Then again, locales in C++ are broken anyway (i.e. the above does the wrong thing for the German text “Maß”, regardless of locale). – Konrad Rudolph Mar 18 '14 at 13:38
@japreiss The primary problem with using the version that does not take a locale is that it is the carry over from the C standard library, which takes, and returns, an `int` with very specific criteria. If `char` is signed on a given platform, the call to `int std::toupper(int)` can lead to undefined behavior. – Zac Howland Mar 18 '14 at 13:44
1

Thanks, I guess my incorrect comment turned out to be a good learning experience :) – japreiss Mar 18 '14 at 13:49

Returning null char array with rewriting Java's String.toUpperCase()

1 Answers1