4
\$\begingroup\$

For this project, I do not have access to Boost or C++11.

Note: I updated the question's description to clarify what I am doing. I have not changed the code though, so no answer has been invalidated.

I have to deal with functions that return error codes.
When one of these functions fails in exceptional cases, I would like to construct an error message and throw a custom exception. Constructing an error message is rather cumbersome though because different functions in different contexts have varying information that would be useful to pass.

An example would be:

Let's say I'm trying to read data from an XML file and this is my interface:

// In header file

struct FooData
{
    std::string strName ;
    std::vector <int> vecScores ;
};

struct FooDataException : public std::runtime_error
{
    FooDataException (const std::string &strWhat) : std::runtime_error (strWhat) {} 
}

class FooDataLoader
{
public:
    FooDataLoader () ;
    ~FooDataLoader () ;

    std::vector <FooData> LoadFooData (const std::string &strPath) const ;
};

In my implementation file, I have two functions that help construct error messages to throw:

static void ThrowFunctionFailure (
    const std::string &strWhere, 
    const std::string &strFunction, 
    const std::string &strMessage)
{
    std::stringstream ss ;

    ss << "In " << strWhere 
        << ", " << strFunction << "failed. "
        << strMessage ;

    throw FooDataException (ss.str ()) ;
}

static void ThrowError (const std::string &strWhere, const std::string &strMessage)
{
    std::stringstream ss ;
    ss << "In " << strWhere << ", an error occurred. " << strMessage ;
    throw FooDataException (ss.str ()) ;
}

Now, these functions remove a lot of code duplication, but I still have to construct an error message at the site:

FooDataLoader::FooDataLoader ()
{
    const HRESULT hr = ::CoInitialize (NULL) ;

    if (FAILED (hr)) {
        std::stringstream ss ;
        ss << "HRESULT: " << hr << "." ;
        throw ThrowFunctionFailure (
            "FooDataLoader::FooDataLoader ()", 
            "::CoInitialize ()",
            ss.str ()
        ) ;
    }
}

FooDataLoader::~FooDataLoader ()
{
    ::CoUninitialize () ;
}

std::vector <FooData> FooDataLoader::LoadFooData (const std::string &strPath) const
{
    if (FileExist (strPath) == false) {
        std::stringstream ss ;
        ss << "The file, " << strPath << ", does not exist." ;
        ThrowError ("FooDataLoader::LoadFooData ()", ss.str ()) ;
    }

    // Some xml library I have to use.
    XmlReader xml ;
    if (xml.read (strPath.data ()) == -1) {
        std::stringstream ss ;
        ss << "File: " << strPath << "." ;
        ThrowFunctionFailure (
            "FooDataLoader::LoadFooData ()", 
            "XmlReader::read ()", 
            ss.str ()
        ) ;  
    }

    // More stuff...
}

I would like to do something like:

ThrowFunctionFailure (
    "FooDataLoader::LoadFooData ()", 
    "XmlReader::read ()", 
    (std::stringstream () << "File: " << strPath << ".").str ()
) ;

But that doesn't work for std::stringstream (at least it doesn't work on my compiler (Visual Studio 2008)).

Based on this answer, I wrote a quick and dirty workaround:

#include <string>
#include <sstream>

template <typename StringStreamT = std::stringstream, typename StringT = std::string>
class QuickStringStream
{
public:
    typedef StringStreamT stringstream_type ;
    typedef StringT string_type ;

    QuickStringStream () {}
    QuickStringStream (const string_type strData) {
        ss_ << strData ;
    }

    template <typename T>
    QuickStringStream& operator<< (const T &tVal) {
        ss_ << tVal ;
        return *this ;
    }

    string_type str () const
    {
        return ss_.str () ;
    }

private:
    stringstream_type ss_ ;
};

This does what I want, but I can't get rid of the feeling that this is a horrible idea.

\$\endgroup\$
  • \$\begingroup\$ In your example at the end, i'd much rather see you provide a function that returns an ostream ref. Then the caller doesn't have to do all that farting around with converting strings to streams and back. It can just write to the stream. (This won't work everywhere, of course...but for I/O, it's a lot less ugly.) \$\endgroup\$ – cHao Apr 8 '14 at 0:17
  • 1
    \$\begingroup\$ Have you considered using std::to_string or boost::lexical_cast to just concatenate strings rather than using streams? (If you're not using C++11 or you don't want to use boost or one function, you can write your own in about 2 lines using streams.) You give up the ability to use manipulators, and it would likely end up being less efficient from all the implicit std::string constructions, but... It would be a lot less verbose and would fit your current usecase. \$\endgroup\$ – Corbin Apr 8 '14 at 2:21
  • \$\begingroup\$ @Corbin No I haven't considered those options. I might just create a custom set of functions that simulate std::to_string. \$\endgroup\$ – jliv902 Apr 8 '14 at 16:37
3
\$\begingroup\$

This strikes me as really fitting better on SO than here. At least IMO, what you need more than a review of this code is an idea of how to replace it with something better.

I guess for the sake of topicality, I'll review at least the interface you've devised. You start with a function that prints a single string to standard output, but it could apparently be just about any function that takes a single string as its input. If you really just wanted to print to standard output, it would be easiest to just use std::cout directly, so I'm going to assume you want to be able to use other functions as well.

Anyhow, you then devise a class that (unlike a stringstream) actually works as a temporary object, then pass its .str() to your function.

To put things simply, that strikes me as a fairly poor approach. In particular, it makes the client code fairly ugly. It still requires that client code to be shaped by the fact that your function accepts only one argument of fixed type, but you want to pass arbitrary arguments to it. As such, my honest conclusion on the code would be that it's ripe (and then a little) to be thrown out and replaced with something better.

Now comes the part of the answer that probably belongs more on SO than here: how to design something better suited to the task, but more starting over from the apparent requirements than really improving the code in your question.

I'd consider two rather different approaches. One would be to simply write a manipulator to invoke your function with the current content of the stringstream. As luck would have it, I posted an answer like this just a couple days ago. This does still involve creating a named stringstream object (can't use a temporary, sorry), streaming things to it, and as the last argument, passing the name of your manipulator that will execute a function on the current content of the stream. An improvement over a plain stream, but I'd guess still not quite what you'd like.

My second (and preferred, if you can use C++11 features) suggestion would be to use a variadic function, something like this:

#include <sstream>
#include <string>
#include <iostream>

template <class T>
std::string stringify(T const &t) {
    // This could use lexical_cast, but for now we'll keep dependencies to a minimum
    std::stringstream b;
    b << t;
    return b.str();
}

template<typename T, typename... Args>
std::string stringify(T arg, const Args&... args) {
    return stringify(arg) + stringify(args...);
}

template<typename F, typename... Args>
void send_cmd(F &f, const Args&... args) {
    std::string buffer = stringify(args...);
    f(buffer);
}

void print(std::string const &s) { std::cout << s; }

int main() {
    std::string three{ " three" };

    send_cmd(print, "one: ", 1, " two plus: ", 2.01, three, '\n');

    return 0;
}

This lets us pass an arbitrary number of arguments, each of arbitrary type. It writes each to a stringstream, then concatenates those strings. The demo code passes a few C-style strings, an int, a double, an std::string and a char, but it should also work with essentially anything else you can insert into a stream (including types using user-defined overloads).

Although I've used a print that works about like your println (just prints out the string), essentially any other function that can be called with a single string parameter should work as well.

When/if you just want to put the items together into a string like your:

std::string s = (
        QuickStringStream <> () << "Hello, " << 5 << " + " << 5 << " = " << 10 << "."
    ).str () ;

...you can just call stringify directly:

std::string s = stringify("Hello, ", 5, " + ", 5, " = ", 10, ".");
\$\endgroup\$
  • \$\begingroup\$ I wish I could use variadic templates, but I can't use C++11 for this project. Boost is also not available. I'll have too look into writing my own manipulator. \$\endgroup\$ – jliv902 Apr 8 '14 at 16:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.