Is it ok to inherit from std::string to provide type consistency?

Question

I want a class to store the name of a team. I could use std::string for this but it is not really semantically correct. An std::string means any string while my team name can't be either empty or longer than 10 characters.

class TeamName {
public:
  TeamName(std::string _teamName) : teamName(std::move(_teamName))
  {
    if (teamName.length() == 0 || teamName.length() > 10) {
      throw std::invalid_argumet("TeamName " + teamName + " can't be empty or have more than 10 chars." );
    }
  }

private:
  std::string teamName;
};

I like this: I'm checking the integrity of the name only once, future developers that never heard of this requirement won't be able to propagate invalid names from their new functions, there is an obvious place in the program logic to place exception handlers, etc.

But this construction means I have to use getters and setters to interact with the name, which pollutes the code a lot. If I inherit from std::string, however, the code barely changes and I can treat TeamName exactly as an std::string

class TeamName : public std::string {
public:
  TeamName(std::string _teamName) : std::string(_teamName)
  {
    if (length() == 0 || length() > 10) {
      throw std::invalid_argumet("TeamName " + teamName + " can't be empty or have more than 10 chars." );
    }
  }
};

Is this bad practice? It feels conceptually correct for me.

What's the way to achieve the consistency I want and still be able to use the wrapper no different from how I use a string?

Answer 1

Actually it depends on how your derived class TeamName is supposed to be used.

All you have to know is that, std::basic_string does not have a virtual destructor (edit: and is not polymorphic by the way).

---

So, the possible issues would appear if you ever try to use polymorphism (i.e. access a TeamName object through a std::string object).

Indeed (example), the following:

std::string * ms = new TestName; // Polymorphism
delete ms;                       // Undefined behaviour

Here, when you call delete, only the destructor of std::string will be called.

The issue is that since the destructor of TestName will never be called in such a case, you open the door to memory leaks.
Moreover deleting the derived object though pointer to base (which does not have a virtual destructor) is Undefined Behaviour. You may be interested to take a look at this SO answer.

This is why, in order to avoid issues, one rule of thumb is to never inherit from a base class that does not have a destructor marked as virtual.

---

But, if you don't intend to use polymorphism at all, it should be fine to make TestName inherit from std::string.

Edit: But I would not recommend you to do so, because you can't prevent anyone that would use your code in the future to misuse your class (e.g. giving it to a function that expects a std::string& for example, as it is polymorphism too).

---

EDIT: _{(details about why we can't use polymorphism)}

Of course, the lack of a virtual destructor is not all of the story. We cannot use polymorphism because std::string is not polymorphic.

Indeed, in order to use polymorphism, the base class needs to be polymorphic.
In other words, the base class needs at least one virtual member function (which can be the destructor of course).

If this requirement is not filled, no vtable (virtual table) will be emitted for this structure.
Consequently, if we try to use polymorphism anyway, whether it is through pointer or through reference, the real object type and implementation will never be found if we try to access it through a non-polymorphic base (dynamic type resolution is impossible without a vtable).
Moreover, an other consequence is that we will become subject to object slicing too (everything about the derived object members will be lost/sliced away).

Answer 2

One potential problem with the inheritance is that it lets people treat your TeamName as a normal std::string. Example:

class TeamName : public std::string {
public:
    TeamName(std::string _teamName) : std::string(_teamName)
    {
        if (length() == 0 || length() > 10) {
            throw std::invalid_argument("TeamName " + *this + " can't be empty or have more than 10 chars.");
        }
    }
};

void appendToString(std::string& s)
{
    s.append(" this is a long text, and the resulting teamname is no longer valid...");
}

int main()
{
    TeamName t = std::string("CoolTeam");
    std::cout << "The teamname is: " << t << std::endl;

    appendToString(t);
    std::cout << "And now it is: " << t << std::endl;
}

Output:

The teamname is: CoolTeam
And now it is: CoolTeam this is a long text, and the resulting teamname is no longer valid...

It is possible to protect your inherited TeamName from all illegal changes - just overload .append and all other functions that can do anything to invalidate it. But with inheritance the problem becomes finding all the functions that have to be checked/restricted, and then writing an overload that handles it.

EDIT: Not true, as pointed out in the comments, member functions in std::string are not marked as virtual. This means that if you access a TeamName through a pointer or ref to std::string then you will get the std::string implementation, and no amount of overloading will be able to protect your TeamName from modifications.

On the other hand, with a private string people can only touch your data through the functions you let them access.

Answer 3

I'm checking the integrity of the name only once, future developers that never heard of this requirement won't be able to propagate invalid names from their new functions, there is an obvious place in the program logic to place exception handlers, etc.

It then is a bad idea to inherit the class with public because this will expose all string manipulation functions, and you would need to make sure that none of them would break your constraints, like:

TeamName test;
test.append( … ); // could make the name longer then 10
test.clear(); // name is 0

You could overwrite each of those current functions and add a check for constraints, but what if you forgot one?

Or you inherit with visibility protected and only expose the functions you need, but then it is just like a wrapper, so you didn't gain anything over an inheritance.

So I see more cons about inheriting from string in that case then benefits.

Answer 4

std::string is not meant to be inherited from publicly. You can inherit privately, but private inheritance is more or less the same as composition and has no big advantage over it.

That being said, it is not the worst practice (even public inheritance can be ok when used with caution), but I think you are overcomplicating the matters. One main job of a class is to ensure that its invariants hold at any time. If you have a Team that has a name as member and that name has certain invariants (not longer than 10 characters, no empty string), it is also completely fine to have a std::string member and let the Team take care of those constraints.

Sometimes you only need to check constraints once. Suppose a Team gets a name when constructed and after that the name cannot change. In such situation, the following is a pattern I use sometimes:

#include <string>
#include <iostream>
#include <stdexcept>
struct TeamName {
    std::string value;
    TeamName(std::string value) : value(value) {
        if (value.length() == 0 || value.length() > 10) {
            throw std::invalid_argument("TeamName " + value + " can't be empty or have more than 10 chars." );
        }
    }
    operator std::string() { return value; }
};

struct Team {        
    Team(TeamName name) : name(name) {}
    private:
    std::string name; 
};

int main() 
{
    try {
        Team x{std::string("")};
    } catch (std::exception& e) { std::cout << e.what(); }

}

Neither the user of the Team nor the Team itself has to bother with validating the name. Also both work with a plain string. The TeamName is merely used for validation.

Answer 5

In C++17, you could use std::string_view as target type of an implicit conversion instead:

class TeamName {
  std::string val;
public:
  TeamName(std::string name): val(std::move(name)) { /* snip */ }

  operator std::string_view() const {
    return val;
  }
};

This lets you use TeamName everywhere you could use a std::string_view, while disallowing modification (so your invariant on the length holds after object creation). Of course, that requires the parts of your code that should consume the object to know about std::string_view.

If std::string_view is not feasible for you, you could define the conversion to const std::string& instead.