HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

More portable toLower() implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationmoretolowerportable

Problem

I am challenging myself to try to attempt to write a function that is as efficient, portable and failsafe as possible. The function is very simple and just converts a string (or wstring) to lowercase.

Are there any improvements you think I could make and any errors I have done?

#ifdef UNICODE
typedef std::wstring tstring;
typedef std::wstring::npos tnpos;
#else
typedef std::string tstring;
//typedef std::string::npos tnpos;  // throws compiler error?!
#endif

tstring toLower(const tstring& str)
{
    // Alternative 1: Returns a copy of a local string so not that efficient.

    unsigned int len = str.length();    // use unsigned int so I only have 4 bytes as opposed to 16 for int
    tstring lower(len, 'a');            // ensure string is correct size to avoid dynamic resizing. Reserve at construction; performing 2 steps at once defintion and reserving - is faster right?
    // or use lower.reserve(len);

    for (unsigned int i=0; i<len; i++)  // iterate using indexes. Maybe iterators could be more fail safe/efficient/etc.?
        lower[i] = tolower(str[i]);     

    return lower;
}

void toLower(const tstring& inStr, tstring& outStr)
{
    // Alternative 2: Have user define string and pass by reference (this would be faster?)

    unsigned int len = inStr.length();  // use unsigned int so I only have 4 bytes as opposed to 16 for int. Store length in local variable because we look it up again in the below for loop
    outStr.resize(len);                 // ensure string is correct size to avoid dynamic resizing.

    for (unsigned int i=0; i<len; i++)  // iterate using indexes. Maybe iterators could be more fail safe/efficient/etc.?
        outStr[i] = tolower(inStr[i]);
}

Solution

typedef std::wstring::npos tnpos;


std::string::npos and std::wstring::npos are static constant integer members of std::string and std::wstring, respectively. They are not types and they cannot be typedef'd.

unsigned int len = str.length();


This is incorrect. std::string::length() has a return type of size_t std::string::size_type which may be larger than an unsigned int depending on your platform and compiler. Other class methods will use the same type for length/position/indexing, so you should make this change for all the other uses of unsigned int.

// or use lower.reserve(len);


No, please don't. That reserves space for the string, but it does not resize it. There is a resize() method.

Since you said you're able to use C++11 features, as long as they're enabled, the first version of the function (the one that returns a string) will work without copying. It will use the move constructor. So you don't have to use the cumbersome version with an input and output parameter. For more information see http://blog.smartbear.com/c-plus-plus/c11-tutorial-introducing-the-move-constructor-and-the-move-assignment-operator/

Code Snippets

typedef std::wstring::npos tnpos;
unsigned int len = str.length();
// or use lower.reserve(len);

Context

StackExchange Code Review Q#86822, answer score: 5

Revisions (0)

No revisions yet.