3
\$\begingroup\$

I need to convert the ENUM to a respective string value for which I came up with two approaches and I'm trying to see why would a second approach be better than the first one if even in terms of performance and practicality?

Both approaches work. The second requires additional work of having to create an array of struct whereas the first approach merely uses a switch case with hardcoded string values of the corresponding enum.

First approach

enum class Num : uint32_t
{
    One,
    Two,
    Three
};

string EnumToStr(Num num)
{
    switch (num)
    {
        case Num::One:
            return "One";
            break;
        
        case Num::Two:
            return "Two";
            break;
        
        case Num::Three:
            return "Three";
            break;
        
        
        default:
            return "";
    }
}

int main() 
{
    cout << EnumToStr(Num::One) << endl; // ONE
    return 0;
}

Second Approach

#define ENTRY(x) {x, #x}

enum class Num : uint32_t
{
    One,
    Two,
    Three,
    MAX
};


struct Lookup
{
    Num num;
    std::string_view str;
};

constexpr static Lookup table[] =
{
    ENTRY(Num::One),
    ENTRY(Num::Two),
    ENTRY(Num::Three)
};

string_view GetStr(Num num)
{
    uint32_t entryIndex = static_cast<uint32_t>(num);
    uint32_t maxNum = static_cast<uint32_t>(Num::MAX);
    
    if (entryIndex < 0 || entryIndex > maxNum)
    {
        return "";
    }
    return table[entryIndex].str;
}

int main() 
{
    cout << GetStr(Num::One) << endl;   // Num::ONE
    return 0;
}

An additional thought that #define ENTRY(x) {x, #x} may be not a recommended approach in C++. Are there better ways around it?

Edit:

The following is what I came up with for the memory consumption for respective functions. It seems the second approach consumes more owing to the struct table in particular.

Second Approach:

  • Looking at the asm, GetStr() consumes (0x401147- 0x401120)+1 = 40 bytes
  • sizeof(table) = 72 bytes
  • total = 72 + 40 = 112 bytes

First Approach:

  • Looking at the asm, SwitchCase() consumes (0x40116d- 0x401120)+1 = 78 bytes

However, doing the following resulted in Second Approach consuming lesser bytes than the First:

  • If const char * is used instead of string_view

  • Adding more enum entries

{gcc12.1 + O3} 166 bytes of First Approach vs 96 bytes of Second Approach (#bytes are calculated at the bottom comments)

So with more enum entries, does the Second Approach outperform the First Approach?

Is this one way of determining the performance of functions?

\$\endgroup\$
3
  • 1
    \$\begingroup\$ using namespace std;? std::endl? You don't have to break out all the bad ideas. \$\endgroup\$ Jul 31 at 14:10
  • \$\begingroup\$ @Deduplicator I understand the importance of std:: & I assure you I’m using it in the production :) \$\endgroup\$
    – xyf
    Jul 31 at 17:18
  • 4
    \$\begingroup\$ Best also avoid it when inviting scrutiny, like here. Do you also know why you should avoid the latter? \$\endgroup\$ Jul 31 at 17:20

3 Answers 3

1
\$\begingroup\$

The const char * approach that you tested is the most performant way to do it, and it is more size efficient than using either std::string_view or std::string, if you are not using the features of string_view or string there is no reason to add the overhead involved.

The Lookup struct you are creating in the second method isn't necessary for a number of reasons, the first is that the code never uses the Num portion of the struct, the second is that you could use std::pair instead.

To answer your comment, yes, because it is an O(1) lookup. This is a size versus speed trade off, however, the executable size of the function will remain the same no matter how large the enum or the array of const char * grow, the executable size of the switch statement will grow with each new case statement. Each new case statement will add a delay the possible solution.

About Deduplicator's comment, prefer std::cout << "\n"; over std::cout <<std::endl; for performance reasons, std::endl calls fflush() which makes a system call, generally this isn't necessary.

\$\endgroup\$
9
  • \$\begingroup\$ Thanks. I was hoping an example would be given as a part of the solution too. I know std::map & std::unordered_map but are you implying to store the enum,string pair instead and use O(1) lookup time to retrieve the respective string value? An example shall clear things up even more \$\endgroup\$
    – xyf
    Jul 31 at 17:20
  • \$\begingroup\$ @xyf You might want to look at the revised answer. \$\endgroup\$
    – pacmaninbw
    Aug 1 at 13:54
  • \$\begingroup\$ Array has a O(1) lookup as well (no linear search is required as shown in the examples) and offers better cache locality than unordered_map. Plus there’s extra overhead associated with hashing of a value which isn’t the case with arrays. I need reasons as to why would you prefer hashtable over array? And again, please provide a code to support your answer \$\endgroup\$
    – xyf
    Aug 1 at 14:16
  • \$\begingroup\$ @xyf I agree with you that an array is a better approach then a map. I changed my answer because of your tests, at first I thought you were asking a how to question in disguise. \$\endgroup\$
    – pacmaninbw
    Aug 1 at 14:18
  • 1
    \$\begingroup\$ @xyf The exact opposite. \$\endgroup\$
    – pacmaninbw
    Aug 1 at 14:40
1
\$\begingroup\$

I'm going to ignore all the asm considerations because the compiled code will change based on (among other factors):

  • the compiler
  • the compiler version
  • the compiler flags used
  • optimizations
  • inlining
  • caching
  • the operating system
  • all the other code in the program

In either approach, the compiler could inline everything so that the only space taken up is by the strings themselves. In fact, that's exactly what happens. These two approaches actually result in the same asm when you use the simplest code for both approaches.

First approach:

C++ code:

#include <cstdint>

enum class Num : uint32_t
{
    One,
    Two,
    Three
};

const char* EnumToStr(Num num)
{
    switch (num)
    {
        case Num::One:   return "One";
        case Num::Two:   return "Two";
        case Num::Three: return "Three";
    }
}

asm (godbolt link):

EnumToStr(Num):
        mov     edi, edi
        mov     rax, QWORD PTR CSWTCH.1[0+rdi*8]
        ret
.LC0:
        .string "One"
.LC1:
        .string "Two"
.LC2:
        .string "Three"
CSWTCH.1:
        .quad   .LC0
        .quad   .LC1
        .quad   .LC2

Second approach:

C++:

#include <cstdint>

enum class Num : uint32_t
{
    One,
    Two,
    Three,
};

constexpr static const char* table[] =
{
    "One",
    "Two",
    "Three"
};

const char* GetStr(Num num)
{
    return table[static_cast<uint32_t>(num)];
}

asm (godbolt link):

GetStr(Num):
        mov     edi, edi
        mov     rax, QWORD PTR table[0+rdi*8]
        ret
.LC0:
        .string "One"
.LC1:
        .string "Two"
.LC2:
        .string "Three"
table:
        .quad   .LC0
        .quad   .LC1
        .quad   .LC2

Both approaches are converted to an array lookup. The only difference is the name of the array. So, the only question is what code is easiest to read, write, and understand. The question of whether to return const char*, std::string_view, or std::string depends on how you expect the function to be used. If I had to guess, std::string_view is probably best since it is lightweight (consisting of two pointers) and interacts naturally with std::string.

Even including the checks for valid enums results in the same asm:

Scoped enums

The purpose of enum class is to create named constants with no implicit conversions to or from the underlying integer type. This allows you to simplify your code because you don't have to handle out-of-bounds values. The only way to get undefined values of Num is to use static_cast<Num>, and all uses of static_cast deserve close scrutiny due to the loss of type information.

First approach

This approach is the simplest and the one I like better. It doesn't need any other data besides the function itself. The compiler will generate the lookup table, so why write it yourself? Also, the compiler will help you keep the function and the enum synchronized.

Since a break after a return is unreachable, we can make this function more compact.

string EnumToStr(Num num)
{
    switch (num)
    {
        case Num::One:   return "One";
        case Num::Two:   return "Two";
        case Num::Three: return "Three";
        default:         return "";
    }
}

The default: case here is unreachable and not necessary, but not all compilers will allow the switch statement without a default: to compile without warnings. The default: case does protect against accidentally skipping Num values, since returning an empty string should have visible consequences in your program. If you ever delete entries from Num the compiler will tell you that, for example, Num::Two is no longer defined.

Second approach

Keeping in mind the purpose of scoped enums, the following check is unnecessary:

    uint32_t entryIndex = static_cast<uint32_t>(num);
    uint32_t maxNum = static_cast<uint32_t>(Num::MAX);
    
    if (entryIndex < 0 || entryIndex > maxNum)
    {
        return "";
    }

The only way the conditional could evaluate to false is if somebody writes auto x = static_cast<Num>(42); GetStr(x);. The check for valid conversions should happen at the point of the static_cast, so the check inside GetStr() and the entry Num::MAX should be deleted. Also, the fact that you cast to an unsigned integer means the entryIndex < 0 check cannot return false.

string_view GetStr(Num num)
{
    return table[static_cast<uint32_t>(num)].str;
}

There's another problem with this. Your second approach includes an extra entry in the enum: Num::MAX. This has a value of 3 and is an out-of-bounds index for table. Even the function with the check would have allowed a call of GetStr(Num::MAX) to access the table with an invalid index. The check should have been if(entryIndex >= maxNum) { return ""; }.

Since you only care about the string representation, I don't see why the Lookup struct has the Num value. It's less typing (and one less function macro) to represent the table as

constexpr static const char* table[] =
{
    "Num::One",
    "Num::Two",
    "Num::Three"
};

Now, the GetStr() function can look like this:

string_view GetStr(Num num)
{
    return table[static_cast<uint32_t>(num)];
}

The downside of this approach is that you have less protection if you don't keep the table and the Num enum synchronized. If you have fewer entries in table than Num, your program will at-best crash after trying to access data beyond the end of the array. If you have more entries in table than Num, then those extra entries uselessly take up space in the program. This is why I prefer the first approach.

\$\endgroup\$
15
  • \$\begingroup\$ 1) I agree with the condition in Second approach to check whether index < 0 is redundant cause of unsigned int, however like you said, index > enum entry could be passed in via static_cast from the caller which may result in problems, hence the need for if (i >= Num::MAX). 2) lookup tables are generated in both the approaches but memory consumed by the first approach is way more than second approach when enum entries are increased. Shouldn't that be taken into account? \$\endgroup\$
    – xyf
    Aug 1 at 16:15
  • \$\begingroup\$ 1) That's true, but it doesn't help if the table is shorter than the enum. Also, why would a static_cast<Num> ever result in an invalid enum value? Anything that is static_cast to an enum needs to be checked for validity. The best place to do this is right where the static_cast occurs. Otherwise, every function that takes a Num will have to do this check. Write your program is such a way that you can always assume that a Num is valid. 2) Where would more memory be consumed? All the strings have to be stored somewhere in the program in both cases. \$\endgroup\$
    – Mark H
    Aug 1 at 17:28
  • \$\begingroup\$ @xyf Approach one with more entries. And approach two with more entries. Still the same asm. Both require more memory to store all the strings. \$\endgroup\$
    – Mark H
    Aug 1 at 17:29
  • \$\begingroup\$ 1) i'm not saying static_cast<Num> would result in an invalid enum, but moreso the condition would be required to ensure the caller does pass the index value that's within the table size. Check the potential issue if index isn't verified: cplayground.com/?p=weasel-crocodile-magpie 2) of course memory is required in both cases but my concern was about who consumes more \$\endgroup\$
    – xyf
    Aug 1 at 18:02
  • 1
    \$\begingroup\$ @xyf Let me say again. Studying the compiler output for an isolated function is not a good way to predict performance. In the context of a full program, the complier may generate asm that is completely different from what you see now. The best way to write code is to write simple code that is easy for you to understand. You should only optimize code after you profile the code and prove that this specific function is a bottleneck. \$\endgroup\$
    – Mark H
    Aug 2 at 5:45
1
\$\begingroup\$

Another way to solve this problem is by using X macros. These allow you to define the list of enum names and their values exactly once, and then create both the enum type and the reverse mapping from that list. For example:

#define NUM_LIST \
    X(One, 1) \
    X(Two, 2) \
    X(FourtyTwo, 42)

enum class Num: uint32_t
{
    #define X(name, value) name = value,
    NUM_LIST
    #undef X
};

string EnumToStr(Num num)
{
    switch (num)
    {
        #define X(name, value) case Num::name: return #name;
        NUM_LIST
        #undef X
    default:
        return {};
    }
}

Once you have written this, then adding a new enum value only requires you to add a line to the definition of NUM_LIST.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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