Feature Description and Review: CyCommandHandler

Overview

Example

Consider the example below. We have two plugins: MyPlugin and YourPlugin. Hypothetically, MyPlugin provides some sort of analysis algorithm that takes a network, a list of nodes, and an iteration parameter and returns a score and a value for each node. YourPlugin wants to use that algorithm and visualize the results in some way. Here is the code for MyPlugin: 

public class MyPlugin extends CytoscapePlugin {
   public MyPlugin() {
      // Plugin initialization
      try {
        // You must reserve your namespace first
        CyCommandNamespace ns = CyCommandManager.reserveNamespace("my algorithm");

        // Now register this handler as handling "analyze"
        CyCommandHandler myHandler = new MyHandler(ns);
      } catch (RuntimeException e) {
        // Handle already registered exceptions
      }
   }

   // Use a separate class so we can extend AbstractCommandHandler
   class MyHandler extends AbstractCommandHandler {
      protected MyHandler(CyCommandNamespace ns) {
          super(ns);
          addArgument("analyze", "network", "current");
          addArgument("analyze", "nodeList");
          addArgument("analyze", "interations", new Integer(2));
      }
      public String getHandlerName() { return "analyze"; }
      public CyCommandResult execute(String command, Map<String, Object>args) 
                             throws CyCommandException {
          // Assuming we've implemented arguments as Tunables
          return execute(command, createTunableCollection(args));
      }
      public CyCommandResult execute(String command, Collection<Tunable>args)
                             throws CyCommandException {
          // Execution code goes here...
      }
   }
 }

The code for YourPlugin is even easier... 

public class YourPlugin extends CytoscapePlugin {
    public YourPlugin {
        // Plugin initialization.  Note: we don't want to look for MyPlugin yet.  That should
        // wait until we actually want to use it.  This avoids errors that result from the
        // arbitrary loading order of plugins.
    }

    public void doWork() {
        Map<String, Object> args = new HashMap();
        args.put("iterations", new Integer(10));
        try {
            CyCommandResult result = CyCommandManager.execute("my algorithm", "analyze", args);
            // Visualize data from result
        } catch (CyCommandException e) {
            // Handle exception
        }
    }
}

Code Layout

All code for the CyCommands implementation is in src/cytoscape/command and is part of the cytoscape.command package. There are no hooks anywhere else in the Cytoscape core that depend on this package. The package provides two interfaces:

In addition to the two interfaces, there are four classes:

And finally, there is a single command implementation for the help function: HelpCommand.

Code Review Comments

Piet

Looks good, something to look forward to for plugin developers (although this should be covered for 3.0 also). I especially like the Tunables mechanism as a model for this

Even more impressive when I studied the actual code (made my remarks above obsolete ;-) Some points about the code:

I'm not an architect but imho the CyCommandHandler interface should be pretty strictly defined to force communication between plugins. Or am I missing something? 

protected void addArgument(String command, String vKey) {
                addArgument(command, vKey, null);
        }

* I think you're right whereas the "help" functionality should be subsumed by the "shell"; so lift the HelpCommand out of command

MikeSmoot

In general I think the concept of CyCommands is a reasonable solution to facilitate plugin communication. I do, however, have serious problems the design of CyCommandHandlers.

The fundamental problem is that the CyCommandHandler interface couples two distinct functions: a command container that manages independent commands and the implementation of individual command behavior (e.g. load network, destroy network). This is to say that CyCommandHandlers violate the single responsibility principle and do not cleanly separate concerns. These classes are forced to do many things that are unrelated and should not be grouped into the same class, which is a problem for several reasons:

A slightly more esoteric concern is that the CyCommandHandler approach also violates Effective Java guideline #50, which recommends avoiding the use of Strings when other types are more appropriate. In the current CyCommandHandler design, the only thing binding the behavior of the CyCommandHandler methods (e.g. execute, getDescription, getTunables, etc.) is the String command name. This means that the command string is only coupled to the actual command behavior by convention and the diligence of the plugin writer. From a practical perspective, plugin writers must go to great lengths to ensure that their use of string commands is consistent throughout the CyCommandHandler class. This is not difficult, just tedious and error prone.

All of this goes towards making the code less modular, harder to maintain, more brittle with respect to change.

The solution I'm proposing is instead of having one CyCommandHandler combine multiple commands into a single class, create a separate CyCommand class for each command. Instead of a command class serving as both a container of multiple commands and the commands themselves, a class simply represents one command and all of the container behavior is moved to the CyCommandManager. When commands do need to share code, an abstract base class can capture similar behavior. This apprach, which we've been calling "One Command, One Class" (OCOC), adheres to the single responsibility principle, Effective Java recommendation #50 and also more closely mirrors the intent of the Command design pattern. OCOC alleviates all of the points mentioned above:

Each point helps create code that is more modular, easier to maintain, and more robust to change.

Finally, an implementation of this approach is available in the attached tar files. command.tgz contains the core API and coreCommands2 shows how some some of the core commands could be ported.

Funding for Cytoscape is provided by a federal grant from the U.S. National Institute of General Medical Sciences (NIGMS) of the Na tional Institutes of Health (NIH) under award number GM070743-01. Corporate funding is provided through a contract from Unilever PLC.

MoinMoin Appliance - Powered by TurnKey Linux