trjhtr

The reason I created this is because I wanted my string values to be type safe and I wanted to decrease the amount of bad data that has crept into my system due to hard coded values in the past. There are different ways to solve this issue, like a lookup table with a key value pair but this is not a direction I can move into at the time. So here is my design and I would love some feedback.

Here is my working sample: https://onlinegdb.com/rympFKEaf

#include <iostream>

class EnumStringContainer

protected:
 EnumStringContainer(const std::string &_str);

public:
 bool operator==(const EnumStringContainer&rhs) const;

public:
 std::string ToString() const;
 const char* ToCharArray() const;

private:
 std::string str;
;

EnumStringContainer::EnumStringContainer(const std::string &_str)

 this->str = _str;


bool EnumStringContainer::operator==(const EnumStringContainer &rhs) const

 return this->ToString() == rhs.ToString();


std::string EnumStringContainer::ToString() const

 return this->str;


const char* EnumStringContainer::ToCharArray() const

 return this->str.c_str();



// Usage
class Color : public EnumStringContainer

private:
 Color(const std::string &color);

public:
 static const Color Red;
 static const Color Blue;
 static const Color Green;
;

// Constructor is private because I don't want new "random" values being created.
Color::Color(const std::string &color) : EnumStringContainer(color)



const Color Color::Red("Red");
const Color Color::Blue("Blue");
const Color Color::Green("Green");

int main()

 Color color = Color::Red;
 return 0;

Design Review

I am not sure why you would want to do this.

enum Colours Red, Green, Blue;

Is much better and type safe.

This code does not have any type safety and thus does not give me any confidence in its usage. Also the comparison is a string compare (not very efficient). Also completely different types are comparable and do not generate a compiler error.

class Color : public EnumStringContainer /* STUFF */
class TypesOfCoral: public EnumStringContainer /* STUFF */

if (Color::red == TypesOfCoral::redCoral) 
 std::cout << "It workedn";

Also there is a slight chance that different types could get a positive match!!!!!

The only reason I can see to use this code is for streaming an enum string, but there is no mechanism to convert that stream back into the correct type so you can't read it back in. So that is a fail (and there are better ways to do that).

Code Review

protected:

Protected! Not sure that is ever useful. But I see you are trying to say this class never stands alone and always has to be inherited from. Think I would prefer the curiously recurring template pattern to achieve that result. But Tomato/Tomato (you have to hear it with me changing accent).

Sure the standard constructor. But in most situations are you not going to construct this with a string literal. Which means you are creating several string objects and copying around a lot of data.

 EnumStringContainer(const std::string &_str);

I move use the new move semantics to prevent the copying.

Sure:

 bool operator==(const EnumStringContainer&rhs) const;

But with == usually comes !=.

Sure:

 std::string ToString() const; // But why return by value.

By returning by value you are forcing a copy.

Can really do this wrong.

 const char* ToCharArray() const;

But not sure why you would need it.

The only reason to use the two above functions is so that you can serialize your enum's easily. So why not have conversion operators or stream operators rather than explicit get the underlying structure.

Also this allows you to stream your object out but not in.

Don't use this->

 this->str = _str;

This is considered good in Java but in C++ it is considered bad practice.

The only reason you need to use this-> is to distinguish between member and local variables. Which means you have local variables with the same name as members, shadowing the members.

Shadowing variables is a real no-no. Because the compiler can't tell when you accidentally do it wrong (using the local and not the member). But you can get the compiler to tell you when you shadow a member. Thus by never shadowing (and getting the compiler to error when you do shadow) your code is actually better as it will have less potential errors in it from using the wrong variable.

In constructors use the initializer list:

EnumStringContainer::EnumStringContainer(const std::string &_str)

 this->str = _str;

By doing it this way. You force the code to initialize the member with the default constructor. Then you reinitialize it with the assignment operator. Best to just initialize it once.

EnumStringContainer::EnumStringContainer(const std::string& str_)
 str(str_)

Put the & next to the type

In C the * usually goes next to the variable. But in C++ the * and & go next to the type. This is because type information is much more important than in C and this is information about the type.

// So rather than this
EnumStringContainer::EnumStringContainer(const std::string &_str)

// I expect to see:
EnumStringContainer::EnumStringContainer(const std::string& _str)

Most counter arguments against this go along the lines.

// Here p and x are different types.
int* p,x;

And I will go yes. But show me a coding standard that allows you to define two variables on the same line. Nobody allows this in modern standards. Its one line per variable;

int* p;
int x;

Your operator==() is inside your API. I see little use in call ToString().

 return this->ToString() == rhs.ToString();

Member naming

Generally the standard is user defined types have an initial capitol letter. While variables and members have an initial lower case letter. Thus allows us to quickly determine what is a type and what is a function or variable.

Prefer not to use _ as a prefix

The rules for using _ as a prefix are quite complicated (ok only a tiny bit complicated). But the problem is most people don't know them. So best to stay away from using _ as a prefix.

In your code you don't do it wrong so there is nothing wrong with your code. But I bet you don't know why.

Now there is no restrictions on using _ as a suffix.

// I would avoid the underscore here.
EnumStringContainer::EnumStringContainer(const std::string &_str)

// prefer the suffix version as there are no restrictions.
EnumStringContainer::EnumStringContainer(std::string const& str_)

I mention this last as it is just one of those quirks and it does not actually break your code. see: What are the rules about using an underscore in a C++ identifier?