Java enum constructor call in foreach loop

Question

Can somebody help me understand why the output of the following code, in java:

public class Main {

    public static int position1, position2;

    private enum Direction{
        North(1,1),
        South(-1,-1),
        West(-1,0),
        East(1,0);

        Direction(int i, int j){
            position1=i;
            position2=j;
        }
    }

    public static void main(String[] args){       

        for(Direction d: Direction.values()){
            System.out.println(d + " has coordinates (" + position1 + "," + position2 + ")");
        }

    }  
}

The output is:

North has coordinates (1,0)
South has coordinates (1,0)
West has coordinates (1,0)
East has coordinates (1,0)

This is, the constructor is called always with the arguments of the element East. Why is this happening? I can't seem to find this explained anywhere.

Answer 1

position1 and position2 are static, and on the Main class. That means they're global there's just one version of each field for anyone who uses Main. What you want are instance fields on Direction. There's more info at the Java tutorial on member fields.

private enum Direction {
    public int position1, position2;
    ...
}

You need to remove the static keyword, so that each Direction has its own position1 and position2 values. You may also want to mark them as final, which means they can only be set once (that's a good general practice, for various reasons: makes it easier to reason about state, catches bugs like this where you accidentally overwrite values, makes for thread-safe code, etc).

When you access them, you need to specify which instance you want to get the position for:

d.position1

(instead of just position1)

The reason this happens is that when you first use the Direction enum, the JVM loads each of its four values. To do that, each value runs its constructor. For instance, when it loads North it runs North(1, 1) which sets position1 = 1; position2 = 1. Then, when it loads South it runs South(-1, -1), which then sets position1 = -1; position2 = -1;. Since there's only one version of each field (that's what static does), this South constructor simply overwrites the values that the North constructor wrote. The end result is that you'll get the position1 and position2 values for whichever constructor ran last. Since the enum's values are loaded in the order they appear in the code, that constructor is the one for the last value: East.

Answer 2

Yes. You are using static fields when your intention is clearly that they be instance fields. The values passed to your constructor are all over-written with the last values used.

Remove "static" and move them inside the enum:

private enum Direction{
    North(1,1),
    South(-1,-1),
    West(-1,0),
    East(1,0);

    public int position1, position2;

    Direction(int i, int j){
        position1=i;
        position2=j;
    }
}

Now refer to the fields of the enum instance:

public static void main(String[] args){       

    for(Direction d: Direction.values()){
        System.out.println(d + " has coordinates (" + d.position1 + "," + d.position2 + ")");
    }

}

I would make the fields final - otherwise they can be changed:

public final int position1, position2;

Answer 3

The static isn't your only problem the location of your position fields is incorrect too. Try this:

public class Main {
    private enum Direction{
        North(1,1),
        South(-1,-1),
        West(-1,0),
        East(1,0);

        public final int position1;
        public final int position2;

        Direction(int i, int j){
            position1=i;
            position2=j;
        }                        
    }

    public static void main(String[] args){       

        for(Direction d: Direction.values()){
            System.out.println(d + " has coordinates (" + d.position1 + "," + d.position2 + ")");
        }

    }  
}