views:

62

answers:

4

I have an interface like the following:

package example;
import java.awt.Point;

public interface Thing {
    public enum MovingState {
        MOVING_LEFT,
        MOVING_UP,
        MOVING_RIGHT,
        MOVING_DOWN
    }

    public void setNewPosition(MovingState state);
    public Point getPosition();
}

and an implementation class:

package example;
import java.awt.Point;

public class ThingImpl implements Thing {
    public enum MovingState {
        MOVING_LEFT (-1, 0),
        MOVING_UP (0, -1),
        MOVING_RIGHT (1, 0),
        MOVING_DOWN (0, 1);

        private int x_move;
        private int y_move;

        MovingState(int x, int y) {
            x_move = x;
            y_move = y;
        }

        public int xMove() {
            return x_move;
        }

        public int yMove() {
            return y_move;
        }
    }


    private Point position;

    public void setNewPosition(MovingState state) {
        position.translate(state.xMove(), state.yMove());
    }

    public Point getPosition() {
        return position;
    }
}

The idea is to have MovingState in ThingImpl extend MovingState from the Thing interface (thus separating the actual implementation of MovingState from the interface).

This doesn't work though - the MovingState enum in ThingImpl shadows the definition in the interface instead of extending it, then the compiler complains that ThingImpl is not abstract and does not override abstract method setNewPosition(Thing.MovingState) in Thing.

Is there an actual way to do what I'm trying to achieve? Or does Java simply not have this capability?

A: 

What you are attempting is an antipattern. Constants should not be defined in an interface.

http://en.wikipedia.org/wiki/Constant_interface

I would move the constants out of your interface, or simply make them methods, and let your implementations define them by implementing those methods...

hvgotcodes
-1 - you're not correct, he's not doing the anti-pattern. What he's doing is stupid, but it's not the constant interface pattern specifically because his interface *defines behavior*, and does not serve to just hold constant values. Defining constants on an interface is fine IFF they're to be used in conjunction with the behavior of the interface.
MetroidFan2002
yeah i guess that is correct and i deserve the vote down. however, the approach is a step in the wrong direction and i think my advice about making his MOVING_LEFT type constants into methods (possibly put into an abstract class) is still good...
hvgotcodes
+3  A: 

You can't extend an enum because it's final.

You may want to read Effective Java 2nd Edition, Item 34: Emulate extensible enums with interfaces. Essentially this boils down to the enum itself that implements Something.

polygenelubricants
Only half true with the can't extend. Each of the enum values can be an anonymous class which extends your enum class.
josefx
I don't have access to this book, but from what I can tell from the sample code, this item doesn't cover the same thing I'm trying to achieve.Item 34 seems to cover extending enums to add new constants, but my aim is only to add member functions. The enum in my base class (Thing) contains all of the required constants. The functionality is what I was hoping to hide in the implementation class (ThingImpl).
MatthewD
@Matthew: I don't think an `enum` is appropriate for defining some abstract type; an `interface` is more appropriate. Perhaps you can have an `interface OrthogonallyMovable`, with `moveUp()`, `moveDown()`, etc, and have a particular implementation that uses `enum`.
polygenelubricants
+1  A: 

polygenelubricants has given you your answer. I'd like to add something to it, because it was an issue that I ran into.

Let's say you have a interface that has many implementations. One method in the interface takes in an enum as an argument. Ideally you would like to have specific sets of enumerated values per implementation. However, since you can't extend enums you can't create a "base" enum and extend from that. The naive approach would be to have a "god" enum that maintains the complete set of enumerated values. But there is a better way. You can use what is known as a marker interface. Effective Java 2nd Ed. talks about this as well. A marker interface will not contain any method declarations, but simply marks a class as being a certain type.

So you can define a marker interface and have all your enums implement that interface. This ties into making the enum extensible, because if you define an interface that your enums extend, you've automatically marked your enums as being a certain type.

This way you can keep all the specific enumerated values along with their specific implementations (separation of concern).

For your case you can do something like this:

public interface MovingInterface {    
   int xMove();
   int yMove();
}

and then:

public enum MovingState implements MovingInterface {
    MOVING_LEFT (-1, 0),
    MOVING_UP (0, -1),
    MOVING_RIGHT (1, 0),
    MOVING_DOWN (0, 1);

    private int x_move;
    private int y_move;

    MovingState(int x, int y) {
        x_move = x;
        y_move = y;
    }

    public int xMove() {
        return x_move;
    }

    public int yMove() {
        return y_move;
    }
}
Vivin Paliath
+1  A: 

What you really want to do is remove the enum declaration from your "ThingImpl" class and move all of it (including its constructor and getters) into the Thing interface.

Make your fields final in the enum to remember that they shouldn't be touched.

In this fashion, anything wishing to use the Thing interface must use the enumeration defined on your interface - your problem is that you're effectively defining it twice, but it should either be on the interface (which is fine if it will only be used for this interface) or as a public level enum Java file (using public enum instead of public class). You'd make it a public enum if something other than your interface could reasonably expect to use it - Map.Entry, in my opinion, is a bad nested interface because other classes have use for a key/value pair external to a map and thus it should be its own interface, but we have to live with it :(

The idea is to have MovingState in ThingImpl extend MovingState from the Thing interface (thus separating the actual implementation of MovingState from the interface).

I don't think this is really your idea - I think the behavior you've specified on the interface of Thing is fine, you really don't want to touch the MovingState enum as it's fine as it is. If you think something needs a different implementation of MovingState, however, you can make it implement an interface called MovingState and thus you could rename your enum DefaultMovingState. It's your choice.

Your MovingState interface would simply have the getters you're exposing in MovingState right now. Two methods.

MetroidFan2002