Is using enums to filter attributes considered a code smell?

Question

I was doing some reading here and one of the suggested answers states:

In short: Don't try to decide how an object might react to some action from the outside, tell it what happened, and let it deal with it.

This other answer suggests:

But in general, I recommend trying to achieve orthogonality, your code stays more maintainable if enums do not have implicit, hidden dependencies to something like a class hierarchy, that will become error prone sooner or later.

The OP was attempting to use enums to replace instanceof.

Suppose if I have the following GameWeapon class:

class GameWeapon(ABC):

    # imports and other methods left out. 

    def __init__(self, name, required_strength, damage, attributes):
        self._name = name
        self._required_strength = required_strength
        self._damage = damage
        self._attributes : List[Attributes] = attributes

    def contains_attribute(self, attribute):
        return attribute in self._attributes 

and Character class:

class Character(ABC):

    def __init__(self, name, strength, attributes):
        self._name = name
        self._strength = strength
        self._attributes = attributes
        self._inventory = []
        self._found_attributes = []
        self._cannot_equip = False
        self._equipped = True

    def add(self, game_object):
        self._inventory.append(game_object)

    # I may obviously want to do more checks, such as required strength,
    # if I currently have another weapon equipped, etc..
    def try_equip(self, game_object):
        if self._check_for_conflicting_attributes(game_object):
            return self._cannot_equip
        return self._equipped

    def _check_for_conflicting_attributes(self, game_object):
        for attrib in self._attributes:
            if game_object.contains_attribute(attrib):
                self._found_attributes.append(attrib)
        return len(self._found_attributes) > 0

and Main

def main():

    wizard_attributes : List[Attributes] = []
    wizard_attributes.append(Attributes.LongBlade)
    wizard_attributes.append(Attributes.Reloadable)

    wiz = Wizard("Gandalf", 100, wizard_attributes)

    sword_attributes : List[Attributes] = []
    sword_attributes.append(Attributes.LongBlade)
    sword = Sword("Standard Sword", 5, 10, sword_attributes)

    # Will print false
    print(wiz.try_equip(sword))


if __name__ == "__main__":
    main()

Suppose if I don't want my wiz to use a sword, I filter on the enum Attribute.LongBlade. I don't decided what to do outside of the Wizard class, I try to equip it, and let the class decides if it can't or can, and I don't use an enum to model my inheritance hierarchy.

Given the quotes above, is using an enum in this way acceptable?

Answer 1

There is a constant tug of war between various design principles and the winner is usually determined by the specific needs of your application. The "characters wielding weapons" problem has been explored extensively and there's an excellent series of articles by Eric Lippert on this topic.

The general consensus is that the rule systems in such games are fairly complex, so it gets rather unwieldy (pardon the pun) to model all these rules as part of class hierarchies. Instead it helps to recognize that these rules are first-class citizens and model them accordingly.

The way you've modeled weapon attributes as an enum is a step in the right direction, and you might want to go a step further and extract the rules for wielding weapons into their own Rule classes. This leads to a more flexible design where Character, Weapon etc. simply describe the attributes of those entities, and the rules can change independently.