Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Void value as return parameter

I have this interface:

public interface Command<T> {
    T execute(String... args);
}

it works fine for most uses. But when I try to model a command that have only side effects (e.g. without return value) I'm tempted to write:

public class SideEffectCommand implements Command<Void> {

    @Override
    public Void execute(String... args) {
        return null; // null is fine?
    }
} 

Is this a common problem? Are there best practices to model Commands with and without return value?

I've tried with this adapter but I think this is not optimal for several reasons:

public abstract class VoidCommand implements Command<Void> {

    @Override
    public Void execute(String... args) {
       execute2(args);
       return null;
    }

    public abstract void execute2(String... args);
}
like image 370
dfa Avatar asked Jul 24 '09 11:07

dfa


4 Answers

I would stick to using Void explicitly. It's easy to see what is going on without another class involved. It'd be nice if you could override a Void return with void (and Integer with int, etc), but that is not a priority.

like image 192
Tom Hawtin - tackline Avatar answered Oct 07 '22 06:10

Tom Hawtin - tackline


Here's a best-of-multiple-worlds implementation.

// Generic interface for when a client doesn't care
// about the return value of a command.
public interface Command {
    // The interfaces themselves take a String[] rather
    // than a String... argument, because otherwise the
    // implementation of AbstractCommand<T> would be
    // more complicated.
    public void execute(String[] arguments);
}

// Interface for clients that do need to use the
// return value of a command.
public interface ValuedCommand<T> extends Command {
    public T evaluate(String[] arguments);
}

// Optional, but useful if most of your commands are ValuedCommands.
public abstract class AbstractCommand<T> implements ValuedCommand<T> {
    public void execute(String[] arguments) {
        evaluate(arguments);
    }
}

// Singleton class with utility methods.
public class Commands {
    private Commands() {} // Singleton class.

    // These are useful if you like the vararg calling style.
    public static void execute(Command cmd, String... arguments) {
        cmd.execute(arguments);
    }

    public static <T> void execute(ValuedCommand<T> cmd, String... arguments) {
        return cmd.evaluate(arguments);
    }

    // Useful if you have code that requires a ValuedCommand<?>
    // but you only have a plain Command.
    public static ValuedCommand<?> asValuedCommand(Command cmd) {
        return new VoidCommand(cmd);
    }

    private static class VoidCommand extends AbstractCommand<Void> {
        private final Command cmd;

        public VoidCommand(Command actual) {
            cmd = actual;
        }

        public Void evaluate(String[] arguments) {
            cmd.execute(arguments);
            return null;
        }
    }
}

With this implementation, clients can talk about a Command if they don't care about the return value, and a ValuedCommand<T> if the need a command that returns a certain value.

About the only reason not to go with using Void straight up is all the unsightly return null; statements that you will be forced to insert.

like image 23
John Calsbeek Avatar answered Oct 07 '22 05:10

John Calsbeek


It looks fine to me. As others said, Void was originally designed for the reflection mechanism, but it is now used in Generics quite often to describe situations such as yours.

Even better: Google, in their GWT framework, use the same idea in their examples for a void-returning callback (example here). I say: if Google does it, it must be at least OK.. :)

like image 4
Aviad Ben Dov Avatar answered Oct 07 '22 04:10

Aviad Ben Dov


This is not a common problem. The issue you need to address is the expectation of your interface. You're combining the behavior of a non-side effect interface with an interface that allows side effects.

Consider this:

public class CommandMonitor {

    public static void main(String[] args)  {       
        Command<?> sec = new SideEffectCommand();       
        reportResults(sec);     
    }   

    public static void reportResults(Command<?> cmd){

        final Object results = cmd.execute("arg");
        if (results != null ) {
            System.out.println(results.getClass());
        }
    }
}

There is nothing wrong with using <Void> as the template type but allowing it to mix with implementations for "Command<T>" means some clients of the interface may not expect a void result. Without changing the interface, you've allowed an implementation to create an unexpected result.

When we pass around sets of data using the Collection classes, my team agreed to never return null even though it's fine syntactically. The problem was that classes that used the return values would constantly have to check for a void to prevent a NPE. Using the code above, you would see this everywhere:

    if (results != null ){

Because there is now way to know whether the implementation actually has an object or is null. For a specific case, sure you'd know because your familiar with the implementation. But as soon as you start to aggregate them or they pass beyond your coding horizon (used as a library, future maintenance, etc.) the null problem will present itself.

Next, I tried this:

public class SideEffectCommand implements Command<String> {

    @Override
    public String execute(String... args) {
        return "Side Effect";
    }

}

public class NoSideEffectCommand implements Command<Void>{
    @Override
    public Void execute(String... args) {
        return null;
    }   
}
public class CommandMonitor {

    public static void main(String[] args)  {       
        Command<?> sec = new SideEffectCommand();
        Command<?> nsec = new NoSideEffectCommand();

        reportResults(sec.execute("args"));
        reportResults(nsec.execute("args")); //Problem Child
    }   

    public static void reportResults(Object results){
        System.out.println(results.getClass());
    }

    public static void reportResults(Void results){
        System.out.println("Got nothing for you.");
    }
}

Overloading didn't work because the second call to reportResults still called the version expecting an Object (of course). I contemplated changing to

public static void reportResults(String results){

But this illustrated the root of the problem, your client code starts having to know implementation details. Interfaces are supposed to help isolate code dependencies when possible. To add them at this point seems like bad design.

The bottom line is you need to use a design which makes clear when you expect a command to have a side effect and to think through how you would handle a set of commands, i.e. an array of unknown commands.

This may be a case of leaky abstractions.

like image 3
Kelly S. French Avatar answered Oct 07 '22 04:10

Kelly S. French