C++ string_cast<> template function

Question

In C++, to simplify string conversion between std::string and std::wstring, I created the following utility template functions:

#pragma once

#include <vector>
#include <string>
#include <cstring>
#include <cwchar>
#include <cassert>

template<typename Td> 
Td string_cast( const char* pSource, unsigned int codePage = CP_ACP );

template<typename Td> 
Td string_cast( const wchar_t* pSource, unsigned int codePage = 1200 );

template<typename Td> 
Td string_cast( const std::string& source, unsigned int codePage = CP_ACP );

template<typename Td> 
Td string_cast( const std::wstring& source, unsigned int codePage = 1200 );

template<>
std::string string_cast( const char* pSource, unsigned int codePage )
{
    assert( pSource != 0 );
    return std::string( pSource );
}

template<>
std::wstring string_cast( const char* pSource, unsigned int codePage )
{
    assert( pSource != 0 );
    std::size_t sourceLength = std::strlen( pSource );
    if( sourceLength == 0 )
    {
        return std::wstring();
    }

    int length = ::MultiByteToWideChar( codePage, 0, pSource, sourceLength, NULL, 0 );
    if( length == 0 )
    {
        return std::wstring();
    }

    std::vector<wchar_t> buffer( length );
    ::MultiByteToWideChar( codePage, 0, pSource, sourceLength, &buffer[ 0 ], length );

    return std::wstring( buffer.begin(), buffer.end() );
}

template<>
std::string string_cast( const wchar_t* pSource, unsigned int codePage )
{
    assert( pSource != 0 );
    size_t sourceLength = std::wcslen( pSource );
    if( sourceLength == 0 )
    {
        return std::string();
    }

    int length = ::WideCharToMultiByte( codePage, 0, pSource, sourceLength, NULL, 0, NULL, NULL );
    if( length == 0 )
    {
        return std::string();
    }

    std::vector<char> buffer( length );
    ::WideCharToMultiByte( codePage, 0, pSource, sourceLength, &buffer[ 0 ], length, NULL, NULL );

    return std::string( buffer.begin(), buffer.end() );
}

template<>
std::wstring string_cast( const wchar_t* pSource, unsigned int codePage )
{
    assert( pSource != 0 );
    return std::wstring( pSource );
}

template<>
std::string string_cast( const std::string& source, unsigned int codePage )
{
    return source;
}

template<>
std::wstring string_cast( const std::string& source, unsigned int codePage )
{
    if( source.empty() )
    {
        return std::wstring();
    }

    int length = ::MultiByteToWideChar( codePage, 0, source.data(), source.length(), NULL, 0 );
    if( length == 0 )
    {
        return std::wstring();
    }

    std::vector<wchar_t> buffer( length );
    ::MultiByteToWideChar( codePage, 0, source.data(), source.length(), &buffer[ 0 ], length );

    return std::wstring( buffer.begin(), buffer.end() );
}

template<>
std::string string_cast( const std::wstring& source, unsigned int codePage )
{
    if( source.empty() )
    {
        return std::string();
    }

    int length = ::WideCharToMultiByte( codePage, 0, source.data(), source.length(), NULL, 0, NULL, NULL );
    if( length == 0 )
    {
        return std::string();
    }

    std::vector<char> buffer( length );
    ::WideCharToMultiByte( codePage, 0, source.data(), source.length(), &buffer[ 0 ], length, NULL, NULL );

    return std::string( buffer.begin(), buffer.end() );
}

template<>
std::wstring string_cast( const std::wstring& source, unsigned int codePage )
{
    return source;
}

Are there any bugs or further optimizations?

Answer 1

While your approach is simple and straightforward to implement there are some important drawbacks to realize.

First, it doesn't take advantage of the power templates offer. Since you're specializing for every possible usage of string_cast there's no opportunity for the compiler to generate code for you. A consequence of this is that you have a lot of 'clipboard heritance', copy and pasting the same function and changing parts of it to do what you want. This is a form of code duplication.

The approach does not lend itself to extendibility. What happens if you want to add support for another string type later on? The combination of functions you have to write would explode!

So there're clearly opportunities for some major improvement. Let see how we can refactor this so that it better adheds to DRY. If you take a step back and think about how string_cast is used you'll find there are really just 3 scenarios it has to support:

• cast to the same string type.
• cast to a different string type.
• cast from a raw pointer representation into a string type.

Each of these cases can be handle by writing a template for them. Starting with the string_cast function that acts as an interface:

template <typename Td, typename Ts>

string_cast now takes 2 template parameters. Keeping your naming convention, I use Ts to indicate the source type. (TO and FROM are probably better names.)

Td string_cast(const Ts &source)
{

We use type deduction to identify what we're casting from.

    return string_cast_imp<Td, Ts>::cast(source);

Once we know what type Td and Ts is we delegate to string_cast_imp and the appropriate template will be instantiated.

}

---

Let's handle the easy case first:

template <typename Td>
struct string_cast_imp<Td, Td>
{
    static const Td& cast(const Td &source)
    {
        return source;

For casting to the same string type, we don't need to do anything. Just return back what was given. Since this is nothing more than a pass-through returning by reference is ok. string_cast will make a copy before going out of scope since it's return by value.

    }
};

---

Now for the important case, the reason for writing string_cast in the first place! The basic process is the same, only certain aspects are different:

• conversion function used. eg. WideCharToMultiByte vs MultiByteToWideChar
• buffer type used. eg. vector<char> for string vs vector<wchar_t> for wstring
• string type returned. That's captured by our template parameter Td so we don't have to worry about this as much.

You can extract those differences into a trait-like policy class.

template <typename Td, typename Ts>
struct string_cast_imp
{
    static Td cast(const Ts &source)
    {
        int length = string_traits<Ts>::byte_convert( CP_ACP, source.data(), source.length(), 
                                                      NULL, 0 );
        if( length == 0 )
        {
            return Td();
        }

Here I removed the string.empty() check since it's not really needed. If the string is empty length will be 0 anyway so this is properly handled.

        vector< typename string_traits<Td>::char_trait > buffer( length );

Here we use our policy class to tell us the proper character-type to use for our buffer. If Td = string then string_traits<Td>::char_trait will be a char. If it's a wstring then string_traits<Td>::char_trait will evaluate to a wchar_t.

        string_traits<Ts>::byte_convert( CP_ACP, source.data(), source.length(), 
                                                 &buffer[ 0 ] , length );

Similiarly, byte_convert acts as a wrapper to the correct byte function to call. This attrib is captured by our policy class as well.

        return Td( buffer.begin(), buffer.end() );
    }
};

We define our string_traits policies like this:

template <typename T>
struct string_traits;

Declare the general base-form but don't define it. This way if code tries to cast from an illegit string-type it will give a compile error.

template <>
struct string_traits<string>
{
    typedef char char_trait;
    static int byte_convert(const int codepage, LPCSTR data  , int data_length, 
                                                LPWSTR buffer, int buffer_size)
    {

You might want to play around with the parameters it accepts but this should give you the general idea.

        return ::MultiByteToWideChar( codepage, 0, data, data_length, buffer, buffer_size );
    }
};

---

And now for the last case. For raw pointer types we can just wrap it into an appropriate string type and call one of our above string functions. We have to overload string_cast here because our base form accepts a reference type. Since reference types to arrays do not decay into a pointer type, this second template form will specifically handle that case for us.

template <typename Td, typename Ts>
Td string_cast(Ts *source)
{
    return string_cast_imp<Td, typename string_type_of<const Ts *>::wrap >::cast(source);

Notice I'm using const Ts * as template parameter for string_type_of. Regardless of whether Ts is const or not we always use template form <const Ts *> to get the information we need.

}

string_type_of is another policy we define as follows:

template <typename T>
struct string_type_of;

This policy class tells us what string-type to use for a given raw pointer type.

template <>
struct string_type_of<const char *>
{
    typedef string wrap;
};

With this refactor, you've reduced the number of written functions from 8 down to just 4 and eliminated code duplication. Perhaps more importantly adding support for another string-type is significantly easier, just specialized another policy for that string-type and you're done.

Answer 2

Just a stylistic note, take it for what it's worth. I generally prefer to avoid:

if ( usual_case )
{
  // lots of code
}
else
{
  // one line handler
}

and instead prefer to go with less indentation, by handling error cases first:

if ( ! usual_case )
{
  return one_liner;
}
// no need for indentation or braces anymore...

Rewriting one of your functions would look like this:

template<>
std::string string_cast( const wchar_t* pSource, unsigned int codePage )
{
  assert( pSource != 0 );
  const size_t sourceLength = std::wcslen( pSource );
  if( sourceLength == 0 )
  {
    return std::string();
  }

  int length = ::WideCharToMultiByte( codePage, 0, pSource, sourceLength, NULL, 0, NULL, NULL );

  std::vector<char> buffer( length );
  ::WideCharToMultiByte( codePage, 0, pSource, sourceLength, &buffer[ 0 ], length, NULL, NULL );

  return std::string( buffer.begin(), buffer.end() );
}

It's slightly cleaner: fewer lines, fewer braces, less indentation. Not much, only slightly, but it adds up with multiple error-case checks, and multiple if-statements.

Also, I made sourceLength const because:

1. The sourceLength isn't going to change. You're going to initialize it, and use it, but you're never going to change it. Using const enforces that you can't change it, and is considered good practice.
2. Using if (sourceLength = 0) is a classic programming mistake. Testing for equality, ==, and assigning, =, differ only by a single character. That typo has caused numerous bugs.
   • if sourceLength is constant, sourceLength = 0 fails to compile.
   • reversing the comparison and using if (0 = sourceLength) also catches the typo at compile time - but it's a little weird to read. Someone nicknamed this "Yoda conditionals".

So, I made it const to conform to good style, and to catch typo bugs -- especially since I changed the conditional from > to ==.