2

So, I implemented an enumToString function for several enums that I use a lot (often asked in SO: Is there a simple way to convert C++ enum to string?, Easy way to use variables of enum types as string in C?, ...). This makes the error messages WAY easier to debug, but I have to maintain the function to add the values that have no string description sometimes.

My code looks like this:

typedef std::map<my_enum_e, const char *> enum_map_t;
static bool s_enum_map_initialized = false;

static enum_map_t s_enum_strings;

static void s_init_maps()
{
#define ADD_ENUM( X ) s_enum_strings[X] = #X;

    if( s_enum_strings.size() == 0)
    {
        ADD_CLASS( MY_ENUM_1 );
        ADD_CLASS( MY_ENUM_2 );
        /* ... all enums */
    }
    s_enum_map_initialized = true;
}

const char *Tools::enumCString( my_enum_e e )
{
    if( ! s_enum_map_initialized )
    {
        s_init_maps();
    }

    // todo: use the iterator instead of searching twice
    if( s_enum_strings.find(e) != s_enum_strings.end() )
    {
        return s_class_strings[e];
    }

    return "(unknown enum_e)";
}

Now, what I want, is that when I don't find the enum in the map, to return "(unknown enum %d)", e . Which will give me the value of the enum I missed.

This way, even if I didn't add it to the map, I still have its value and I can debug my program.

I can't find a way to do that simply: a stringstream instanciated on the stack will be destroyed right after the return, a static stringstream is not thread-safe, ...

edit: of course, using a std::string as return type would allow me to format it, but I call these functions very often in my code, I figured passing a const char * pointer is faster, since I don't have to push the std::string onto the stack each time.

Any solution?

Community
  • 1
  • 1
Gui13
  • 12,993
  • 17
  • 57
  • 104
  • 1
    Why `const char*` and not `std::string`? If `std::string` then you could use a `std::ostringstream` (or similar). – hmjd Apr 16 '12 at 13:44
  • See my comment to Nick's answer: I *chose* const char * to make the calls faster. – Gui13 Apr 16 '12 at 13:50
  • 2
    You're worried about the performance of your *error messages?* Have you actually measured that error reporting takes a significant fraction of the running time in some at-least-semitypical use case? – Christopher Creutzig Apr 16 '12 at 13:53
  • @Gui13: Using a `switch` instead of a `map` would be faster. *much* faster. And any compiler worth its salt would warn you if an enumerator is missing as long as you don't include a `default` clause. See my answer for generating this switch automatically. – Matthieu M. Apr 17 '12 at 08:04
  • I actually upvoted your answer, although I don't use boost (just look at how much includes you have for a single switch()). This is a nicer solution, since the absence of default will elegantly solve my problem! – Gui13 Apr 17 '12 at 08:20

4 Answers4

4

Return a std::string rather than a char*.

This would allow you to use a std::stringstream to generate your message. The calling site would then just have to use the .c_str( ) member function on std::string to get the C-style pointer (if required).

Nick
  • 25,026
  • 7
  • 51
  • 83
  • Yeahhh... but no, I chose `const char *` because the compiler will simply return the static address of the string, whereas using `std::string` will make the calls to the function WAY heavier. – Gui13 Apr 16 '12 at 13:50
  • And have you written some profiling code to prove this? Or are you just doing pre-emptive optimisation? – Nick Apr 16 '12 at 13:53
  • Take your bet :D More seriously, I use a C-style logging function, so the calls to the functions look like this: `LOG( "Event %s\n", Tools::enumCString( event ) );`. Refactoring ALL my calls to this function to add a `.c_str()` would be tedious. I'd rather find a solution that doesn't involve all this. – Gui13 Apr 16 '12 at 13:56
  • 1
    Can you not add a new `LOG(const char* a_fmt, const std::string& a_msg) { LOG(a_fmt, a_msg.c_str()); }` function? – hmjd Apr 16 '12 at 13:59
  • Okay I thought about your solution, I might simply have the internal storage setupt as a `std::string` and have my `enumCString()` call `enumStdString(e).c_str()`. That's acceptable. – Gui13 Apr 16 '12 at 14:01
3

Personally, I use BOOST :)

Example of use:

SANDBOX_DEFINE_ENUM(MyEnum, (Foo)(Bar)(Team))

Will yield:

struct MyEnum {
  enum Type {
    Foo,
    Bar,
    Team
  };
  static Type const First = Foo;
  static Type const Last = Team;
};

inline char const* toString(MyEnum::Type value) {
  switch(value) {
  case MyEnum::Foo: return "Foo";
  case MyEnum::Bar: return "Bar";
  case MyEnum::Team: return "Team";
  }
  return 0;
}

Code:

#include <boost/preprocessor/cat.hpp>
#include <boost/preprocessor/stringize.hpp>

#include <boost/preprocessor/seq/enum.hpp>
#include <boost/preprocessor/seq/for_each.hpp>
#include <boost/preprocessor/seq/reverse.hpp>
#include <boost/preprocessor/seq/seq.hpp>

#define SANDBOX_DEFINE_ENUM(Name_, Values_)                                     \
  SANDBOX_DEFINE_ENUM_TYPE(Name_, Values_)                                      \
  SANDBOX_DEFINE_ENUM_STRING(Name_, Values_)

#define SANDBOX_DEFINE_ENUM_TYPE(Name_, Values_)                                \
  struct Name_ {                                                                \
    enum Type {                                                                 \
      BOOST_PP_SEQ_ENUM(Values_)                                                \
    };                                                                          \
    static Type const First = BOOST_PP_SEQ_HEAD(Values_);                       \
    static Type const Last = BOOST_PP_SEQ_HEAD(BOOST_PP_SEQ_REVERSE(Values_));  \
  };

#define SANDBOX_DEFINE_ENUM_STRING(Name_, Values_)                              \
  inline char const* toString(Name_::Type value) {                              \
    switch(value) {                                                             \
      BOOST_PP_SEQ_FOR_EACH(SANDBOX_DEFINE_ENUM_TO_STRING_C, Name_, Values_)    \
    }                                                                           \
    return 0;                                              \
  }

#define SANDBOX_DEFINE_ENUM_TO_STRING_C(r, Name_, Elem_)                        \
  case Name_::Elem_: return BOOST_PP_STRINGIZE(Elem_);

Obviously, it only work with "regular" enums, not with custom made ones. But because it's defined in a single place in the code... no maintenance penalty :)

Matthieu M.
  • 287,565
  • 48
  • 449
  • 722
  • +1 for an elegant solution - however it doesn't solve the original problem which was to do with dealing with undefined enum values! – Nick Apr 17 '12 at 07:37
  • @Nick: it does in a way, there is no longer any enumerator that has not matching string. The only problem that may appear is when casting from int to enum, and the `First` and `Last` provide the usable range. – Matthieu M. Apr 17 '12 at 08:03
  • Very handy. Any chance of this working for enumerations where the values are defined explicitly, e.g. `enum Type { Foo = 5, Bar = -2, Team = 19 };` – ulatekh Feb 21 '14 at 18:00
  • @ulatekh: If all versions are explicitly defined, then it can be relatively easy (just augmenting each argument of the sequence to a pair of items); in its crudest form it does imply quite a lot of parentheses though for the caller: `DEF_ENUM(Enum, ((Foo, 5)(Bar, -2)(Team, 19)))` and an extra level of unpacking outside it. At this point, it might be simpler to declare the `enum` manually and extract from the above macro the string generation part. Combined with `-Wswitch` (which warns on missing enumerators) the compiler will check that all enumerators are translated for you! – Matthieu M. Feb 21 '14 at 21:38
0

Try defining a thread-local static variable http://gcc.gnu.org/onlinedocs/gcc-3.3.1/gcc/Thread-Local.html

user1202136
  • 11,171
  • 4
  • 41
  • 62
0

I wouldn't return a value in this case. I would throw an exception and have your message contain the value of your invalid enumerator. To me, an invalid enumerator value would seem to be an error.

If you don't want to do that, then I agree with others that you should be returning a std::string instead.

Anon Mail
  • 4,660
  • 1
  • 18
  • 21