2

I was reading on this SESE page about using a variable to indicate the object type, more specifically, an enum.

The accepted answer states:

When your weapon types enum just mirrors the class hierarchy, then this is a blatant violation of the DRY principle - you encode the same information, the type of a weapon redundantly in two places - one place is the class name itself, one place the enum. That's definitely not just a code smell, that is bad code.

As suggested in the comments of the OP, a Collection containing specific attributes might be better. I like this approach because I could query it to find out more about the object and proceed accordingly.

For example:

public enum GameObjectAttributes{

    Replenishable
    Destructable
    Health
    LongBlade
    ShortBlade
    Mana
    Health
    Potion
    Indestructible
}

Weapon:

public abstract class Weapon 
{
    private List<GameObjectAttributes> gOAttributes;

    // imagine a constructor :D.

    public final boolean containsAttribute(GameObjectAttributes attribute)
    {
        // determine if weapon contains a specific attribute. 
    }
}

Suppose I had a Sword object, and the level of detail allows a Sword to be damaged and broken if hit hard enough, so my Sword object might contain Destructable and LongBlade as attributes.

Question:

Does my code suffer from what the answer in the link warns against, that I'm using a variable to map my class hierarchy? Although it might contain an enum about what the object is, it also contains details that for obvious reasons one enum won't be able to tell you, in my Sword example, that it's Destructable.

2 Answers2

1

If we're basing it on strictly what you have written here, no, there is no violation. However, I have to imagine that you have logic somewhere regarding the fact that it is Destructable or that it is a LongSword, am I correct?

In this case, presumably you're using classes to determine behavior for these attributes or else you're using a gigantic if else chain. In the former case, then yes, you're running into precisely the DRY principle violation in which you referred to. In the latter case, you're not, but don't do it this way either. It's not good practice.

However alternatively what you can do is pass a class to the constructor of your enum directly linking the enum with the class which deals with it. Assuming all these attributes can be succinctly generalized into a common interface, you can apply the attributes to the weapon without ever knowing what enum instance you have. This is ideal.

What you want to avoid are things like:

if(gOAttribute.equals(GameObjectAttributes.Destructable)) {
    // Do destructable logic
} else if (gOAttribute.equals(GameObjectAttributes.LongSword)) {
    // Do long sword logic
} else if (...)
   ...
Neil
  • 22,848
0

Without knowing more of your specific case, I would say yes, you are doing the same thing, you are just making it more complex.

The fundamental problem you are having, is that you are trying to decide how an object would react to some action from outside the object. This is fundamentally not how object-orientation works (or should work). You should ask the object directly to perform the action, and the object will hopefully tell you what the result is if any.

For example: You say a Wizard can't carry Sword. That might be, but the user can still ask the Wizard to try, can't it? So just implement a tryCarry() method for all characters, and let the character decide whether that is possible.

Second example: Not all objects are destructible you said. Still you could implement a takeDamage() for all objects since you obviously want to deal damage to everything, and let the object decide whether it "destructed", or it had no effect.

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.

Robert Bräutigam
  • 12,340