Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is this a good reason for non-polymorphic inheritance?

std::string (as most — if not all — standard classes) doesn’t have any virtual methods, so creating an inherited class with virtual methods will result in UB (due most likely to the destructor). (correct me if I am wrong).

I thought that inheriting without polymorphism it’s ok though, until I read upon the net on this subject.

For instance, in this answer: Why should one not derive from c++ std string class? some arguments are given against this practice. The main reason seems to be the slicing problem which will inhibit the added functionality when the derived object is passed to a function in place of a std::string parameter, thus making the non-polymorphism not logical. The idiomatic C++ way is to create free functions if one wants to extend the functionality of string. And I agree with all that, especially since I am an advocate for free functions instead of monolithic classes.


That being said I think that I found a situation that I think actually warrants the non-polymorphic inheritance from std::string. I will first show what issue I am trying to solve, then I will show the reasons why I think inheriting from std::string is the best solution here.

When porting a function used for debugging purposes from C to C++ I realised there is no way to create formatted strings in C++ (not using C-like string functions e.g. sprintf). I.e.:

C version: void someKindOfError(const char *format, ...);
this would be called like:

someKindOfError("invalid argument %d, size = %d", i, size);

C++ version: void someKindOfError(const std::string &message);
to call this to a similar effect would be:

std::stringstream ss;
ss << "invalid argument " << i << ", size = " << size;
someKindOfError(ss.str());

this can’t be a one liner because the << operator returns an ostream. So this requires 2 extra lines and an extra variable.

The solution I came up with is a class called StreamString that inherits from std::string (actually a templated BasicStreamString that inherits from basic_string<>, but that’t not important) and as far as the new functionality goes it has the operator << that mimics the behaviour of the stringstream operator and conversion to and from string.

So the previous example can become:

someKindOfError(StreamString() << "invalid argument " << i << ", size = " << size);

remember that the parameter type is still const std::string &

The class is created and fully functional. And I found this class very useful in a lot of places when a string is need to be created ad-hoc, without the extra burden of having to declare an extra stringstream variable. And this object can be manipulated further like a stringstream, but it is actually a string and can be passed to functions expecting string.


Why I think this is an exception to the C++ idiom:

  • the object needs to behave exactly like a string when passed to functions expecting a string, so the slicing problem is not an issue.
  • the only (noteworthy) added functionality is the operator << which I am reluctant to overload as a free function for a standard string object (this would be done in a library).

One alternative I can think of is to create a variadic templated free function. Something like:

template <class... Args>
std::string createString(Args... args);

which allow us to call like this:

someKindOfError(createString("invalid argument ", i , ", size = " , size));

One disadvantage of this alternative is the lost of the ability to easily manipulate the string like an stringstream after it’s creation. But I suppose I can create a free function to handle that too. Also people are use with the operator << to perform formatted inserts.


To round up:

  • Is my solution bad practice (or worst) or it is an exception to the C++ idiom and it is OK?
  • If it is bad, what viable alternatives are there? Is createString ok? Can it be improved?
like image 483
bolov Avatar asked May 24 '14 17:05

bolov


2 Answers

You don't need to derive a class from std::string for this. Just create an unrelated class, say StringBuilder that keeps a std::stringstream internally. Overload operator << for this class and add a std::string cast operator.

Something like this should do the trick (untested):

class StringBuilder
{
    std::ostringstream oss;

public:
    operator std::string() const
    {
        return oss.str();
    }

    template <class T>
    friend StringBuilder& operator <<(StringBuilder& sb, const T& t)
    {
        sb.oss << t;
        return *this;
    }
};
like image 173
D Drmmr Avatar answered Oct 16 '22 23:10

D Drmmr


Your StreamString class is OK, in that there do not seem to be any situations where normal usage of it could get you into trouble. Even so, there are a lot of alternatives that might be more appropriate to this situation.

  1. Use a pre-existing library, such as Boost.Format, rather than rolling your own. This has the advantage of being widely known, tested, supported, etc...

  2. Write someKindOfError to be a variadic template, to match the C version, but with added C++ type-safety goodness. This has the advantage of matching the C version, and so being familiar to your existing users.

  3. Give StringStream a conversion operator or an explicit to_string function, rather than inheriting from std::string. This gives you more flexibility to change the implementation of the StringStream at a later stage. (For example, at a later stage, you might decide that you want to use some kind of cacheing or buffering scheme, but this would be impossible if you do not know exactly when the final string will be extracted from the StringStream).

    As it is, your design is conceptually flawed. The only thing that you need is the ability to convert a StringStream to a std::string. Inheritance is an overly heavy-handed way of achieving that goal, when compared to using a conversion operator.

  4. Write your original stringstream code as a one-liner:

someKindOfError(static_cast<std::stringstream &>(
    std::stringstream{} << "invalid argument " << i << ", size = " << size).str());

... well, that's pretty ugly, so maybe not. You should consider it though, if your only reason for not doing this was that you thought it was not possible.

like image 25
Mankarse Avatar answered Oct 16 '22 23:10

Mankarse