Loki-Astari / ThorsMongo

C++ MongoDB API and BSON/JSON Serialization library
GNU General Public License v3.0
316 stars 72 forks source link

Issue with ThorsAnvil_MakeEnum #49

Closed danielaparker closed 5 years ago

danielaparker commented 5 years ago

In traits.h, in the macro ThorsAnvil_MakeEnum, we have

static EnumName getValue(std::string const& val, std::string const& msg) \
{                                                               \
    char const* const* values = getValues();                    \
    std::size_t        size   = getSize();                      \
    for (std::size_t loop = 0;loop < size; ++loop)              \
    {                                                           \
        if (val == values[loop]) {                              \
            return static_cast<EnumName>(loop);                 \
        }                                                       \
    }                                                           \
    throw std::runtime_error(msg + " Invalid Enum Value");      \
}                                                               \

This looks to me like it will only work for enum types with consecutive values. That is, it will work for your test case,

enum RGB { Red, Green, Blue };

but not for something like

enum class FloatFormat {scientific = 1,fixed = 2,hex = 4,general = fixed | scientific};

Note that in the latter instance, the default initialized value FloatFormat() is a valid enum value even though, being 0, there is no name for it in the list, and the named values are not consecutive.

Instead of generating an array of # member, I would suggest generating an array of {EnumName::member, # member} pairs. Then getValue can search for the name and return the corresponding EnumName. I think you also need a fallback if there is no name for the default initialized value EnumName(), perhaps associating it with an empty string, or some other convention.

Also consider something like

enum class ErrorCode {Error1 = 1, Error2};

where ErrorCode() represents success. The standard library std::errc uses this convention. In this case you would want to be able to serialize/deserialize

ErrorCode ec = ErrorCode();

Best regards, Daniel

Loki-Astari commented 5 years ago

@danielaparker That's a good suggestion.

I can do it this weekend. But I am more than willing to accept a pull request. You even have the perfect test case to add to the unit tests.

Loki-Astari commented 5 years ago

Update: https://github.com/Loki-Astari/ThorsSerializer/commit/6ac1586f6e56133ffbad2aa9e747de26c765b6c1 . Homebrew pull-request: https://github.com/Homebrew/homebrew-core/pull/41680