25

I made an enum as:

enum class KeyPressSurfaces {
    KEY_PRESS_SURFACE_DEFAULT,
    KEY_PRESS_SURFACE_UP,
    KEY_PRESS_SURFACE_DOWN,
    KEY_PRESS_SURFACE_LEFT,
    KEY_PRESS_SURFACE_RIGHT,
    KEY_PRESS_SURFACE_TOTAL
};

and later on I attempt to define an array as I typed below, but I received the error, size of array 'KEY_PRESS_SURFACES' has non-integral type 'KeyPressSurfaces'

SDL_Surface*KEY_PRESS_SURFACES[KeyPressSurfaces::KEY_PRESS_SURFACE_TOTAL];

I understand the error fine, but I don't know where to move the KeyPressSurfaces to qualify the constant in the enum.

I also realize I could just use an enum and not an enum class, but I feel like this should work, and I want to learn how to do this.

Any response/advice?

genpfault
  • 51,148
  • 11
  • 85
  • 139
Hayden Piper
  • 353
  • 1
  • 3
  • 9
  • why do you want so much to use an enum to specify the size of an array? The size should be a compile time constant. – 463035818_is_not_an_ai Mar 11 '16 at 09:36
  • 13
    The enum is a compile time constant. – parsley72 Oct 02 '16 at 05:16
  • 1
    @user463035818 when you add more items on enum, the `KEY_PRESS_SURFACE_TOTAL` automatically adjust. It's a good technique actually to use enum for the array size. When it's a constant number, you have to edit all part in the codes that are related to the size of that array specially on some computation where the size-of-the-array is involve. – acegs Aug 20 '18 at 02:20
  • 10
    I'm surprised no one else mentioned this: Given that `enum class`es are scoped, I would've thought half of the point of them is to avoid you having to repeat ugly prefixes like `KEY_PRESS_SURFACE_` on every enumerator. You don't need to protect the global namespace anymore. By thinking you do, now you have to write it *twice*... `KeyPressSurfaces::KEY_PRESS_SURFACE_DEFAULT` Yuk! Just drop the prefix, drop the `ALL_CAPS` too because there are no macros here, drop the plural that IMO is unnecessary and best reserved for instances of collections, and write `KeyPressSurface::default`. Much better. – underscore_d Sep 29 '18 at 09:15
  • 2
    @underscore_d +1 for everything but `KeyPressSurface::default` which will simply not compile. Using Google guidelines you would instead write something like `KeyPressSurface::kDefault` (https://google.github.io/styleguide/cppguide.html#Enumerator_Names) – Andreas Magnusson May 13 '20 at 22:15
  • @AndreasMagnusson Ah, yup, because `default` is a keyword. D'oh! – underscore_d May 14 '20 at 18:26
  • 1
    What is a good reason to use scoped enums instead of unscoped enums when using the values as array indices? If it's about the scope, unscoped enums can be put to a namespace. – Roland Sarrazin Dec 21 '21 at 18:04
  • 1
    @underscore_d I don't know where you got the idea that ALL_CAPS is only for macros. Lots of people use it to make it clear when something is a compile-time constant (as opposed to a variable), which enumerators certainly qualify for. This usage goes way, way back in C++ and C, not to mention existing usage in standard libraries. Many coding standards specify this. Also, as Andreas points out, using ALL_CAPS for these avoids potential collisions with keywords which could result in ugly workarounds. – Some Guy Aug 24 '22 at 23:10

9 Answers9

26

Scoped enums (enum class) are not implicitly convertible to integers. You need to use a static_cast:

SDL_Surface*KEY_PRESS_SURFACES[static_cast<int>(KeyPressSurfaces::KEY_PRESS_SURFACE_TOTAL)];
Simple
  • 13,992
  • 2
  • 47
  • 47
  • 9
    This is right, but this is also ugly, and because those small pieces things added up to make c++ getting unpopular these days – r0n9 Oct 22 '21 at 03:00
  • 1
    There's [std::to_underlying](https://en.cppreference.com/w/cpp/utility/to_underlying) coming in C++23 that will make it clearer. – Vennor Jun 17 '22 at 14:31
24

You can convert your enum to int using template function and you will get more readable code:

#include <iostream>
#include <string>
#include <typeinfo>

using namespace std;

enum class KeyPressSurfaces: int {
    KEY_PRESS_SURFACE_DEFAULT,
    KEY_PRESS_SURFACE_UP,
    KEY_PRESS_SURFACE_DOWN,
    KEY_PRESS_SURFACE_LEFT,
    KEY_PRESS_SURFACE_RIGHT,
    KEY_PRESS_SURFACE_TOTAL
};

template <typename E>
constexpr typename std::underlying_type<E>::type to_underlying(E e) {
    return static_cast<typename std::underlying_type<E>::type>(e);
}


int main() {
    KeyPressSurfaces val = KeyPressSurfaces::KEY_PRESS_SURFACE_UP;
    int valInt = to_underlying(val);
    std::cout << valInt << std::endl;
    return 0;
}

I fount to_underlying function here

Community
  • 1
  • 1
gomons
  • 1,946
  • 14
  • 25
6

You can work on the array:

/** \brief It's either this or removing the "class" from "enum class" */
template <class T, std::size_t N>
struct EnumClassArray : std::array<T, N>
{
    template <typename I>
    T& operator[](const I& i) { return std::array<T, N>::operator[](static_cast<std::underlying_type<I>::type>(i)); }
    template <typename I>
    const T& operator[](const I& i) const { return std::array<T, N>::operator[](static_cast<std::underlying_type<I>::type>(i)); }
};
Stefan
  • 4,380
  • 2
  • 30
  • 33
4

Remove the class keyword or cast explicitly to an integral type.

knivil
  • 787
  • 3
  • 11
4

Building on the other responses, another alternative is a simple templated class that wraps a c-style array. With the EnumArray example below, any enum class with a kMaxValue can be used as the index.

In my opinion, the improved readability is worth the introduction of a template.

template <class IndexType, class ValueType>
class EnumArray {
 public:  
  ValueType& operator[](IndexType i) { 
    return array_[static_cast<int>(i)];
  }

  const ValueType& operator[](IndexType i) const {
    return array_[static_cast<int>(i)];
  }

  int size() const { return size_; }

 private:
  ValueType array_[static_cast<int>(IndexType::kMaxValue) + 1];

  int size_ = static_cast<int>(IndexType::kMaxValue) + 1;
}; 

enum class KeyPressSurfaces {
    KEY_PRESS_SURFACE_DEFAULT,
    KEY_PRESS_SURFACE_UP,
    KEY_PRESS_SURFACE_DOWN,
    KEY_PRESS_SURFACE_LEFT,
    KEY_PRESS_SURFACE_RIGHT,
    KEY_PRESS_SURFACE_TOTAL,
    kMaxValue = KEY_PRESS_SURFACE_TOTAL
};

int main() {
    EnumArray<KeyPressSurfaces, int> array;
    array[KeyPressSurfaces::KEY_PRESS_SURFACE_DEFAULT] = 5;
    std::cout << array[KeyPressSurfaces::KEY_PRESS_SURFACE_DEFAULT] << std::endl;
    return 0;
}
eric
  • 642
  • 1
  • 9
  • 11
3

You can use a namespace and anonymous enum. So you can get rid of these ugly prefixes and use enum items as index.

namespace KeyPressSurfaces
{
    enum
    {
        DEFAULT = 0,
        UP,
        DOWN,
        LEFT,
        RIGHT,
        TOTAL
    };
}

SDL_Surface* KEY_PRESS_SURFACES[KeyPressSurfaces::TOTAL];
1

Use struct members instead of enum.

struct KeyPressSurfaces {
    static constexpr int KEY_PRESS_SURFACE_DEFAULT = 0;
    static constexpr int KEY_PRESS_SURFACE_UP= 1;
    static constexpr int KEY_PRESS_SURFACE_DOWN = 2;
    static constexpr int KEY_PRESS_SURFACE_LEFT = 3;
    static constexpr int KEY_PRESS_SURFACE_RIGHT = 4;
    static constexpr int KEY_PRESS_SURFACE_TOTAL = 5;
};

Alternatively, have them in a namespace with the same logic, where you can benefit from using namespace.

namespace KeyPressSurfaces {
    constexpr int KEY_PRESS_SURFACE_DEFAULT = 0;
    constexpr int KEY_PRESS_SURFACE_UP= 1;
    constexpr int KEY_PRESS_SURFACE_DOWN = 2;
    constexpr int KEY_PRESS_SURFACE_LEFT = 3;
    constexpr int KEY_PRESS_SURFACE_RIGHT = 4;
    constexpr int KEY_PRESS_SURFACE_TOTAL = 5;
}
SDL_Surface* KEY_PRESS_SURFACES[KeyPressSurfaces::KEY_PRESS_SURFACE_TOTAL];
Burak
  • 2,251
  • 1
  • 16
  • 33
0

In addition to the currently accepted answer, you could write a function to get a reference to the surface:

enum class KeyPressSurface
{
    DEFAULT,
    UP,
    DOWN,
    LEFT,
    RIGHT,
    TOTAL
};
// This is using static_cast like the accepted answer
std::array<SDL_Surface *, static_cast<int>(KeyPressSurface::TOTAL)> keyPressSurfaces;

// Function to get a reference to a surface
SDL_Surface *&getKeyPressSurface(KeyPressSurface surface)
{
    return keyPressSurfaces[static_cast<int>(surface)];
}

Now you can cleanly get a surface using the enum class:

// assignment
getKeyPressSurface(KeyPressSurface::UP) = SDL_LoadBMP("lamp.bmp");
// or get a value
SDL_Surface *currentSurface = getKeyPressSurface(KeyPressSurface::RIGHT);
Joe'
  • 376
  • 6
  • 10
-3

Alternatively you can replace your array with a map, which also means you can get rid of KEY_PRESS_SURFACE_TOTAL:

enum class KeyPressSurfaces {
    KEY_PRESS_SURFACE_DEFAULT,
    KEY_PRESS_SURFACE_UP,
    KEY_PRESS_SURFACE_DOWN,
    KEY_PRESS_SURFACE_LEFT,
    KEY_PRESS_SURFACE_RIGHT
};

std::map<KeyPressSurfaces, SDL_Surface*> KEY_PRESS_SURFACES;
parsley72
  • 8,449
  • 8
  • 65
  • 98
  • 2
    This isn't really a general solution to the problem, since std::map will be slower than using an array. – Kef Schecter Nov 18 '17 at 06:10
  • 3
    "Premature optimization is the root of all evil." - there's nothing in the question that suggests speed is an issue, the compiler may be clever enough to handle this and I find this a cleaner solution. – parsley72 Nov 18 '17 at 19:17
  • 20
    I'm pretty sure no compiler (past, present, or future) will turn an std::map into an array. There is also a difference between writing slow code because you don't want to optimize prematurely and writing slow code because you have no idea the construct you're using is slow. I happen to be writing a program myself right now (a chess engine) where the performance hit from using std::map would be utterly brutal. – Kef Schecter Nov 18 '17 at 22:12
  • 1
    @parsley72 that quote is not always accurate. Often the bugs cause by premature optimization is when your optimization target is for speed or memory-usage and you didn't consider other stuffs in your program. There are also other kind of optimization like optimize-for-maintainability-and-flexibility which will largely lessen the bugs and will be good for business in the long run. The production might be slow at first though but it's worth it. – acegs Aug 20 '18 at 02:10
  • 1
    @AngelM1981 Separate from everything else we might debate, there is a definite decrease in readability and quality of style by using `ALL_CAPS` for anything except a macro, especially a variable name (there's some tradition of also using it for enumerator names, but that's still bad IMO). – underscore_d Sep 30 '18 at 15:39
  • @underscore_d I was copying the question's code to make it obvious. – parsley72 Jun 03 '20 at 03:52