Enum or int

[leroy tsruya]

Hi All
I have a question which is actuall a "design" question.
I am creating a small game, and now working on the Attack class.
basically it has couple of attributes there which are all int values (representing attack power, attack pp and so on..) and do not matter for my question.
However, I want to have a member to indicate the attack type (a physical attack, a status attack, etc..).
so take this simple class:

public class Attack {
   int AtkType;
   
   public Attack(/*all attributes...*/, int atkType){
         this.atktype = atktype;
   }
}

now i thought of creating an enum Attacktype for all the types.
and so the constructor would look like:

public Attakc(AttackType atkType){
   ...
}

What do you think would be better? I currently retrieve the data for all attacks using ms access, so in the database the attack type is represented by an int.

Thanks!

[Rob Spoor]

I'd use the enum. With int, what would you do if I provided Integer.MIN_VALUE? Or 123456789? Both are valid ints, but neither is what you expect (I assume).

[David Newton]

+1 for enums. Type safety, additional enum functionality.

[David O'Meara]

+1 again

[leroy tsruya]

Thanks guys!
So you advise that when I read the data from the database I'll have a switch statement like so:

public AttackType setAtkType(int atkType){
AttackType a;//the enum Attacktype
switch(atkType){
 case 1: a = AttackType.PHYSICAL;
            break;
 case 2: a = AttackType.SPECIAL;
            break;
 ....
}
return a;
}

and then assign a to the atkType member of Attack?

[John de Michele]

Leroy:

A couple of things:

1) You really shouldn't shorten your method names. setAttackType() will be easier to understand six months down the line than setAtkType().
2) Why bother translating an int to an AttackType? Why not pass the AttackType directly?

John.

[leroy tsruya]

1) You really shouldn't shorten your method names. setAttackType() will be easier to understand six months down the line than setAtkType().

Great advice, I'll update my code
Thanks.

2) Why bother translating an int to an AttackType? Why not pass the AttackType directly?

The data is stored in a database (ms access).
The attack type field there is an int (1,2,3).
so when i read the data, I read it as an int.
Then, the setAttacType method takes the int and returns the enum AttackType representation for that int.

[John de Michele]

The attack type field there is an int (1,2,3).
so when i read the data, I read it as an int.
Then, the setAttacType method takes the int and returns the enum AttackType representation for that int.

You might consider something like this:

public enum ExampleEnum {

    First(1),
    Second(2),
    Third(3);

    private final int type;

    private static final Map<Integer, ExampleEnum> translationMap = new HashMap<Integer, ExampleEnum>();

    static {
        translationMap.put(First.type, First);
        translationMap.put(Second.type, Second);
        translationMap.put(Third.type, Third);
    }

    private ExampleEnum(final int type) {
        this.type = type;
    }

    public static ExampleEnum getEnum(final int type) {
        return translationMap.get(type);
    }

    public int getType() {
        return type;
    }

}

This will let your enum do the translating, rather than the class that holds the enum.

John.

[Tom Reilly]

Another alternative:

public enum Attack
{
	LeftFlank, RightFlank, FullFrontal, Sneak;
	
	static Attack getAttack(int ordinal)
	{
		for (Attack attack : Attack.values())
		{
			if (attack.ordinal() == ordinal)
				return attack;
		}
		return null;
	}
}

public class AttackTest
{
	public static void main(String[] args)
	{
		System.out.println(Attack.getAttack(0));
		System.out.println(Attack.getAttack(1));
		System.out.println(Attack.getAttack(2));
		System.out.println(Attack.getAttack(3));
		System.out.println(Attack.getAttack(4));
	}
}

And use enum's ordinal() method to store the "enum" in the DB.

[David Newton]

Looping instead of a map; meh.

[John de Michele]

Tom:

I have to agree with David. You really don't gain anything with a loop. In fact, if the enum should grow, the cost of doing a search every time goes up.

John.

[David O'Meara]

David Newton wrote:Looping instead of a map; meh.

That's not very clear.
Personally I think enums are typically small (an assumption in their design switches behaviour at 127 items) so while both approaches have arguments for and against, I would lean towards NOT caching the value unless there was a need to.

[Rob Spoor]

I usually loop as well. In the end, it's a trade off. A loop takes a little longer but requires no extra memory. A map is faster but requires some extra memory. Both the extra time and extra memory are small enough to make both options good.

[David Newton]

Re-meh. I guess I just don't work with so little data that the O(n)/O(1) thing isn't meaningless.

[leroy tsruya]

Wow Thank you all guys!
I got a lot of ideas to think of here!
I'll check your suggestions with my code, and see what suits best!
Thanks

[David Newton]

One thing we're very good at is talking a lot and disagreeing

[Martin Vajsar]

If you really (really!) can afford to use ordinals to store the enums, the following is pretty elegant way to obtain the enum based on its ordinal value:

public enum MyEnum {
    First,
    Second,
    Third;

    private static final MyEnum [] values = values();

    public static MyEnum byOrdinal(int ordinal)
    {
        return values[ordinal];
    }
}

This - in my opinion - beats the hashmap both in the terms of space and speed and is much more concise to code.

However, using ordinals to represent your enums in the database is a grave decision. If you would want to keep backward compatibility in your application, you would have to only add new values, never delete or even reorder them, as this changes the ordinals assigned to individual values.

If you opt for the hashmap, I'd advise to initialize it as follows:

public enum ExampleEnum {  
  
    First(1),  
    Second(2),  
    Third(3);  
  
    private final int type;  
  
    private static final Map<Integer, ExampleEnum> translationMap = new HashMap<Integer, ExampleEnum>();  
  
    static {  
        for (ExampleEnum e : values()) {
            translationMap.put(e.type, e);  
        }
    }  
  
    private ExampleEnum(final int type) {  
        this.type = type;  
    }  
  
    public static ExampleEnum getEnum(final int type) {  
        return translationMap.get(type);  
    }  
  
    public int getType() {  
        return type;  
    }  
}  

That way you do not have to change the initialization if you add or remove a value.

However, if you obtain the numbers form the database and have only a dozen or so values in the enum, a simple loop over the array would probably suffice. The time to obtain a number from the database is certainly several orders of magnitude higher than to loop over some ten items.

[Seetharaman Venkatasamy]

Welcome to JavaRanch martin

[David Newton]

Martin Vajsar wrote:However, using ordinals to represent your enums in the database is a grave decision. If you would want to keep backward compatibility in your application, you would have to only add new values, never delete or even reorder them, as this changes the ordinals assigned to individual values.

It also relies on the ordinals not having any intrinsic values of their own, which isn't always the case--particularly with legacy databases.

The time to obtain a number from the database is certainly several orders of magnitude higher than to loop over some ten items.

Looping is only a reasonable solution if you're doing it a few at a time--if you're iterating over thousands of rows, meh.

[Martin Vajsar]

I've done a little test. Performance of the loop is generally comparable to hashmap for small enums (around the dozen or so I mentioned earlier), but one must avoid to call values() method for every lookup. Given that some setup is needed anyway, the hashmap seems a reasonable solution to me now as it will scale well when there are new values added to the enum.