Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Here is some code I wrote 10 years ago. I'm now reviewing it and there are a lot of things I don't like.

There is possibility for weird behaviour if the template is attempted to be used with a type that I didn't intend. Also I don't like the big macros for DECLARE and DEFINE; and weird compile errors can occur if it is not used in just the right way.

Any suggestions are welcome; I do have C++11 access now (although I didn't when first writing this of course).

The goal of SmartEnum is to:

  1. Provide a bijection (unique two-way mapping) between an enum type and a string.
  2. Provide a bijection between each enum item and a string
  3. Provide a description string for the enum.
  4. Convert an enum value to an integral value
  5. Convert an integral value to an enum value, throwing if no valid enum value exists. This must be the inverse of the previous conversion

The purpose I use SmartEnum for is that I read and write a configuration file in human-readable format, and in the file the enums are referred to by name; and the enum items can also be referred to by name. I include enums in this scheme via the following function:

// Convert to builtin and/or enum automatically!
template<typename T> T to_any(std::string const &s)
{
// Blank string goes to zero. (Config2XML relies on this)
    long v = 0;
    if (!s.empty())
        v = to_integral_type<long>(s, -1);   // does what it sounds like

#pragma option push -w-8008         // compiler complains about condition always false/true
#pragma option push -w-8066
    if (enum_properties<T>::is_enum)
        return to_enum<T>(v);
    else
        return static_cast<T>(v);
#pragma option pop
#pragma option pop
}

// this is needed as the previous function barfs if instantiated with std::string
template<> inline std::string to_any<std::string>(std::string const &s)
    { return s; }

The code for SmartEnum:

#ifndef H_SMART_ENUM
#define H_SMART_ENUM

#include <sstream>
#include <ostream>
#include <stdexcept>
#include <map>
//---------------------------------------------------------------------------
// Smart enums: cast from integer and check range. Make name and description available.
//
    template<typename T> struct enum_properties
    {
        static const long max = 0;
        static const bool is_enum = false;
        static std::string name() { return std::string(); }
        static std::string desc() { return std::string(); }
        static std::string item_name(T t);
    };

    template<typename E> std::string enum_item_name(E e) { return enum_properties<E>::item_name(e); }

    // Retrieve item name if it is specialize, otherwise build a default
    template<typename T> std::string enum_string(T t)
    {
        std::string s = enum_properties<T>::item_name(t);
        if (!s.empty())
            return s;

        std::ostringstream oss;
        oss << "{" << enum_properties<T>::name() << " " << static_cast<long>(t) << "}";
        return oss.str();
    }

    template<typename T> T to_enum(long x)
    {
        if (x < 0 || x > enum_properties<T>::max)
        {
            std::ostringstream oss;
            oss << "Unknown " << enum_properties<T>::desc() << ": " << x;
            throw std::runtime_error(oss.str().c_str());
        }

        return static_cast<T>(x);
    }

#define DECLARE_SMART_ENUM(E) \
    template<> struct enum_properties<E> {  \
        static const bool is_enum = true;   \
        static const long max;              \
        static std::string name();          \
        static std::string desc();          \
        static std::string item_name(E);    \
    };                                      \
    inline std::ostream &operator<<(std::ostream &os, E e) { return os << static_cast<long>(e); }

#define DEFINE_SMART_ENUM(E, E_max, E_desc) \
    const long        enum_properties<E>::max = static_cast<long>(E_max);              \
    std::string enum_properties<E>::desc() { return E_desc; }           \
    std::string enum_properties<E>::name() { return #E; }               \

//---------------------------------------------------------------------------
#endif

Sample usage:

// file.h
enum class Fruit
{
    Apple = 1,
    Orange,
    Lemon,
    maxFruit
};

DECLARE_SMART_ENUM(Fruit);

// file.cpp
DEFINE_SMART_ENUM(Fruit, Fruit::maxFruit - 1, "A special fruit");

std::string enum_properties<Fruit>::item_name(Fruit f)
{
    switch(f)
    {
        #define C(X) case X: return #X
        C(Apple);
        C(Orange);
        C(Lemon);
        #undef C
        default: return "";
    }
}
share|improve this question
    
Have a look here. codereview.stackexchange.com/a/14315/507 you don't need C++11 logic to make this work well. –  Loki Astari Jun 13 at 15:06
    
@LokiAstari some interesting stuff there - although I really don't like the array (in comparison with the switch version): there's possibilities of out-of-bounds access, and it doesn't have any nice way of making sure the strings actually match up with the enum constants. Although maybe those things can be fixed up. –  Matt McNabb Jun 14 at 4:20

1 Answer 1

Since you have access to a C++11 compiler, you should use the type trait std::is_enum in the default version of your template instead of just writing is_enum = false. Also, now, it should be constexpr:

template<typename T> struct enum_properties
{
    static constexpr bool is_enum = std::is_enum<T>::value;
    // ...
};

Now, you don't have any mean to know whether T is a simple enum or a "smart" one though, but the name is_enum was confusing, and since there is such a trait in the standard, your class should reflect its baheviour. Now, we want another mean to differenciate simple enums from smart ones. I propose to add another constant is enum_properties, along the lines of std::numeric_limits<T>::is_specialized. This constant would tell whether enum_properties is specialized for a given type, and the name is less amibuous than the previous one.

template<typename T> struct enum_properties
{
    static constexpr bool is_enum = std::is_enum<T>::value;
    static constexpr bool is_specialized = false;
    // ...
};

#define DECLARE_SMART_ENUM(E) \
    template<> struct enum_properties<E> {  \
        static constexpr bool is_enum = std::is_enum<E>::value; \
        static constexpr bool is_specialized = true; \
        /* ... */ \
    };
share|improve this answer
    
Great, I didn't know about std::is_enum and it never came up when I was searching for how to tell if something was or was not an enum! Also it seems like a good idea to have the is_specialized variable, then I actually don't have to provide all of the other members at all. I can just leave them out and get a compilation error if I accidentally use a non-smart enum where a smart one is expected. –  Matt McNabb Jun 14 at 4:21

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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