3

Can this code be refactored to avoid copying switch statements?

enum class Animal
{ Cat, Dog, Fish};

float GetMaxSpeed(Animal a)
{
 switch (a)
    {
        case Animal::Cat:
            return 30;
        case Animal::Dog:
            return 40;
        case Animal::Fish:
            return 15;
    }
 }

string GetGermanTranslation(Animal a)
{
 switch (a)
    {
        case Animal::Cat:
            return "Katze";
        case Animal::Dog:
            return "Hund";
        case Animal::Fish:
            return "Fische";
    }
 }

Obviously, this is a toy example, my real enum class is much larger.

JFMR
  • 23,265
  • 4
  • 52
  • 76
Ivana
  • 643
  • 10
  • 27
  • 1
    Seems quite simple. Depending on the number of enum values, did you try simply using either a map or an array that enumerates the enum values, and gives the corresponding speed and translation? Or, if the enum values are guaranteed to be numerically consecutive, a simple array of both? No switch statements needed anywhere. – Sam Varshavchik Aug 21 '20 at 10:58
  • 1
    Consider using a different approach: `const AnimalData animalData[] = {{Animal::Cat, 30, "Katze"}, {Animal::Dog, 40, "Hund"}, {Animal::Fish, 15, "Fische"}};` - unfortunately C++ doesn't support the designated initializer syntax, `[Animal::Cat] = {30, "Katze"}` – user253751 Aug 21 '20 at 11:49
  • @user253751 could you elaborate on this, preferably in an answer. – Ivana Aug 21 '20 at 12:14
  • @SamVarshavchik Thanks, that is also very useful. Is there any solution for when there is no direct mapping, for example if maximum speed is unknown? – Ivana Aug 21 '20 at 14:10
  • A map (`std::map`) can be used to filter out sparse indices, but it seems an overkill. You can use `std::optional` for features that might be absent on some cases. Or you can use a `std::bitset` to keep track of existent features. – Red.Wave Aug 21 '20 at 15:03

2 Answers2

6

A more object-oriented approach would consist of relying on polymorphism instead of continuously switching on an enumerator value representing a type.

To follow this approach, first, define an abstract class, Animal, that specifies the member functions its subclasses should implement:

class Animal {
public:
    virtual std::string GetGermanTranslation() const = 0;
    virtual float GetMaxSpeed() const = 0;
    virtual ~Animal() = default;
};

Then, publicly derive from this class and override the virtual member functions GetGermanTranslation() and GetMaxSpeed().

For example, for Cat:

class Cat: public Animal {
public:
   std::string GetGermanTranslation() const override { return "Katze"; };
   float GetMaxSpeed() const override  { return 30; };
};

Then, similarly for Dog:

class Dog: public Animal {
public:
   std::string GetGermanTranslation() const override { return "Hund"; };
   float GetMaxSpeed() const override  { return 40; };
};

You can analogously define the Fish class.

Finally, having a pointer or reference to an Animal object, you just call the corresponding virtual member functions:

void displayAnimal(const Animal& animal) {
   std::cout << "The " << animal.GetGermanTranslation();
   std::cout << " runs at " << animal.GetMaxSpeed() << '\n';
}

As you can see, there is no more switching on an enumerator that represents a type for distinguishing the different animals.

You can call this function, displayAnimal(), with different Animal objects:

auto cat = std::make_unique<Cat>();
displayAnimal(*cat);

auto dog = std::make_unique<Dog>();
displayAnimal(*dog);
JFMR
  • 23,265
  • 4
  • 52
  • 76
  • This is _way_ overkill. IMO an example of how not to do it. The switch is vastly simpler, faster, and easier to maintain. Also your solution is kind of orthogonal anyway; the OP has enum values not instances of things. – Asteroids With Wings Aug 21 '20 at 11:42
  • 2
    @AsteroidsWithWings This approach follows the [Open-closed Principle](https://en.wikipedia.org/wiki/Open%E2%80%93closed_principle) whereas the `switch` doesn't. For example, imagine that you have to **extend** the animals with a *mouse* animal: with the OOP approach, all you need to do is just to create a new class `Mouse` that derives from `Animal` and override the corresponding member functions. On the other hand, with the `switch` approach you will have to go through the whole code looking for all the `switch` statements and add a new case for every switch statement. – JFMR Aug 21 '20 at 11:47
  • 1
    The polymorphic approach is extendible without modification of existing code, whereas the approach based on the `switch` statement requires modification of existing code for extending. The latter it's not only more work to do but also more error-prone. It is also less localized and spread across the whole code. – JFMR Aug 21 '20 at 11:49
  • Incorrect. Compilers warn about missing enumerations in a switch. It's a very, very small job. You are vastly overengineering a very simple problem. – Asteroids With Wings Aug 21 '20 at 11:49
3

If you don't specify otherwise, the first enumerator in an enumerated type has the value 0, and subsequent enumerators have a value that's one more than their predecessor. So with

enum Animal
{ Cat, Dog, Fish};

the value of Cat is 0, the value of Dog is 1, and the value of Fish is 2. To map those values to some other set of values, just use them as an array index:

int max_speed[] = {
    30, 40, 15
};

const char* german_name[] = {
    "Katze",
    "Hund",
    "Fische"
};

Now you can write things like max_speed[Cat] and german_name[Dog].

Note that the string literals are inside double quotes, e.g., "Katze", not single quotes as in the question. Single quotes around multiple characters create a funky thing called a "multicharacter literal". They're pretty much useless.

Pete Becker
  • 74,985
  • 8
  • 76
  • 165
  • This is very useful, but in my real-world case i want the have explicit, readable, mappings between the enums and their properties. – Ivana Aug 21 '20 at 14:08