Java abstract method: is this bad practice?

Question

I the following code:

        public static final Condition.ActionCondition ACTION_CONDITION_ACTIVATE = new Condition.ActionCondition(ACTION_CONDITION_ACTIVATE_ID, "Activate")
        {
            private static final long serialVersionUID = 5660276607942658334L;

            @Override
            public Object action(Object... params)
            {
                return null;
            }
        };

which extends my class Condition. The only difference in these classes is the above abstract method "action". Regular Conditions (the class that it extends) do not have any actions, but are stored in an Conditions class' ArrayList, but that may be beside the point.

The idea behind ActionConditions is that they have an Integer, representing their condition, in this case ACTION_CONDITION_ACTIVATE_ID, a name, in this case "Activate", and finally a method that does something. The method can return anything, do anything it wants, and have anything as its parameters.

I can think of multiple different uses of this class, but the way it is written with the action method, just rubs me the wrong way for some reason. Is there a better way to accomplish what this class does? Or is the class well written enough?

Answer 1

This is a matter of opinion, of course, but since this is a coding style question I suppose it's fine to give an opinion.

Writing a method that can take any parameters and return anything it wants circumvents much that is gained by using a type-safe language like Java. I would wager that there is almost certainly a better way to accomplish your end-goal, other than by writing a method which takes an arbitrary number of Object parameters and returns an Object.

Without knowing much more about what you need to do with such a class, it's hard to make a recommendation, but I have a sense that using generics could probably help you. Instead of passing just an action id to determine what the class would do, the instantiator of the object could also give the required type parameters to determine what the action method would accept and return.

Following your code above, an anonymous class use would look something like this:

    public static final Condition.ActionCondition<MyReturnType,MyParamType1,MyParamType2> ACTION_CONDITION_ACTIVATE = new Condition.ActionCondition<MyReturnType,MyParamType1,MyParamType2>(ACTION_CONDITION_ACTIVATE_ID, "Activate")
    {
        private static final long serialVersionUID = 5660276607942658334L;

        @Override
        public MyReturnType action(MyParamType1 param1, MyParamType1 param2)
        {
            return null;
        }
    };

This assumes an abstract class for ActionCondition declared inside the Condition class something like this:

public static abstract class Condition.ActionCondition<R,P1,P2> extends Condition{
    public abstract R action(P1 param1,P2 param2);
}

This would be assuming you want the action to always accept no more than 2 parameters. You can probably see how you could generalize this to an arbitrarily chosen maximum number of parameters.

Note: From your description, it sounds like you were describing your anonymous class usage as the declaration of an abstract class. This is likely just a minor mis-wording, but this combined with some of your a-typical structure makes me wonder if it's possible you don't have a great understanding of abstract and/or anonymous classes. If my explanation above made 0 sense to you, you might want to try reading about the differences between Nested Classes, abstract classes, and anonymous classes