Jump to content
  • Advertisement
Sign in to follow this  
Storyyeller

How can I improve this code? (C++ enums)

This topic is 2484 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

So in my game I have various secret items, each associated with a name and image. The problem is coming up with a way to automatically assign an index to each one (from 0 to n-1) so that they can be iterated over and used as indicies in a bitmap.

Enums solve the automatic index assignment problem, but they require type casting to iterate over or use as indices. So what should I do? Here's my current implementation. The problem is that I would have to manually change all the values when adding or removing a secret.


[source lang='cpp']typedef unsigned int Secret;

namespace Secrets
{
Secret begin();
Secret end();

Secret POTATO();
Secret NONPOISONOUSMUSHROOM();
Secret TM49();
Secret OBVIOUSSECRET();
Secret DELICIOUSCAKE();
Secret BIGDIPPER();

std::string Name(Secret s);
ImgHandle Image(Secret s);
}[/source]

[source lang='cpp']namespace Secrets
{
const Secret ePOTATO = 0;
const Secret eMUSHROOM = 1;
const Secret eTM49 = 2;
const Secret eOBVIOUS = 3;
const Secret eCAKE = 4;
const Secret eDIPPER = 5;
const Secret eNUMSECRETS = 6;

Secret begin() {return 0;}
Secret end() {return eNUMSECRETS;}

Secret POTATO() {return Secret(ePOTATO);}
Secret NONPOISONOUSMUSHROOM() {return Secret(eMUSHROOM);}
Secret TM49() {return Secret(eTM49);}
Secret OBVIOUSSECRET() {return Secret(eOBVIOUS);}
Secret DELICIOUSCAKE() {return Secret(eCAKE);}
Secret BIGDIPPER() {return Secret(eDIPPER);}

std::string Name(Secret s)
{
switch (s)
{
case ePOTATO: return "Potato Battery";
case eMUSHROOM: return "Nonpoisonous Mushroom";
case eTM49: return "TM49 (Tri Attack)";
case eOBVIOUS: return "Very Obvious Secret";
case eCAKE: return "Delicious Cake";
case eDIPPER: return "Big Dipper";
default: assert(false); return "INVALID SECRET";
}
}

ImgHandle Image(Secret s)
{
switch (s)
{
case ePOTATO: return ImgHandle(ImgDataStruct("Secrets/potato.png",0,99,0));
case eMUSHROOM: return ImgHandle(ImgDataStruct("Secrets/nonpoisonousmushroom.bmp",255,0,255));
case eTM49: return ImgHandle(ImgDataStruct("Secrets/tm49.bmp",0,255,0));
case eOBVIOUS: return ImgHandle(ImgDataStruct("Secrets/obvioussecret.bmp",0,255,0));
case eCAKE: return ImgHandle(ImgDataStruct("Secrets/deliciouscake.bmp",0,255,0));
case eDIPPER: return ImgHandle(ImgDataStruct("Secrets/bigdipper.bmp",0,255,0));
default: assert(false); return ImgHandle();
}
}
}[/source]

Share this post


Link to post
Share on other sites
Advertisement
I the compiler you are using supports the C++11 standard then you can use an enum and set the underlying type of that enum so no casting will be necessary.

Share this post


Link to post
Share on other sites
What about an array of Secret objects?


struct Secret {
enum Index {
mushroom,
cake,
dipper,
/* ... */
count
};

Index idx;
string name;
ImgHandle image;
};

Secret theSecrets[Secret::count] = {
{ Secret::mushroom, "Nonpoisonous Mushroom", ImgHandle( ... ) },
{ Secret::cake, "Delicious Cake", ImgHandle( ... ) }
/* ... */
};


First, it seems to make sense to make some sort of object, since all Secrets seem to have an ID, a description, and an ImgHandle. From here, we can make an array for the type of all the secret-types. Adding or removing a secret requires changing the Secret::Index enum, and will emit a compiler error if you forget to update the array. An array can also be iterated over, and the Index enum doesn't require type casting.

However, this approach requires that the IDs of the items in the array match up. A simple sort on program execution or an assertion could enforce this.

Another alternative would be to use an associative container: map<Secret::Index, Secret>

Share this post


Link to post
Share on other sites
Try to store the item types in a dynamic array and load them from a file for the ability to load a mod from a folder. Try to represent their differences using numbers and boolean flags instead of hard coded behaviours. For example: Instead of naming all the items that can be used as a weapon, store a boolean property called "is weapon" so that an item type can be removed or duplicated without much effort. If the weapon have properties for the type of bullet used, refer to another list of ammunition types.

The same problems are common when designing databases without redundancy.
http://en.wikipedia....e_normalization

Share this post


Link to post
Share on other sites
If you're indexing by enum then the std::map is the more appropriate solution.

Share this post


Link to post
Share on other sites

If you're indexing by enum then the std::map is the more appropriate solution.

This.
[source]
#include <iostream>
#include <map>
#include <string>


enum Secret
{
POTATO,
MUSHROOM,
TM49,
OBVIOUS,
CAKE,
DIPPER,
NUMSECRETS,
};


int
main(int argc, char* argv)
{
std::map<Secret, std::string> mapping = {
{ POTATO, "Potato Battery" },
{ MUSHROOM, "Nonpoisonous Mushroom" },
{ TM49, "TM49 (Tri Attack)" },
{ OBVIOUS, "Very Obvious Secret" },
{ DIPPER, "Big Dipper" },
};

for (auto p = mapping.begin(); p != mapping.end(); ++p)
{
std::cerr << p->first << ": " << p->second << "\n";
}
}
[/source]
Compiled and tested on Ubuntu 10.04 with g++ 4.5.2 (this is an older compiler does not support the current C++ standard, so no lambdas or [font="Courier New"]for (auto p: mapping)[/font] range iteration).

If you replace the [font="Courier New"]std::string[/font] as the [font="Courier New"]map[/font] value with a struct, you can get both the name and image.

Share this post


Link to post
Share on other sites
It's an associative container, which is what should be used when you're not indexing by integer. Otherwise I'd recommend a std::vector. An array should typically be avoided if possible. It's a C holdover; using the STL is the proper way to approach containers in C++.

Share this post


Link to post
Share on other sites

If you're indexing by enum then the std::map is the more appropriate solution.

It seems a bit odd to use a binary tree when an vector/array is basically a perfect hash for enums. Just don't encode meanings into the integer representation of your enums and there's no problem.

The C++ practice of giving enums special integer values is kind of weird. It almost feels like enums are a hack that paper over a hole in the language. Particularly when they're used as flags.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!