Command Handling

As the title explains, I'm wondering about command handling. I've handled commands in various ways, but my most favorite way is via a separate CommandExecutor class. Because I was curious, I searched and discovered this thread about command handling!
As the thread above mentions annotating command information and implementing a command listener is "better" for bigger plugins. I've reached a point where some of my plugins I'm developing have too many commands, but I've always handled them in one class (for the most part). So ...

How do you guys handle commands in your plugins? Is annotating command classes suitable?

Thanks for the feedback. Because I'm a neat-freak, I must have everything nice, organized, and readable!


By commands, I mean sub-commands like /example <test> and /example <test2> ! NOT /example <test> and /anotherExample <test2> !

Sry if this thread sounds confusing. I'm was a little tired as I was writing this!

 

Whenever I have a lot of commands (I once got a request do one with 50 different commands!) I just registered each individual command separately.

getCommand("aCommand").setExecutor(new ClassThisCommandWillExecuteTo(), this);

I eventually just created a whole new package and put them all in there. That's how I would do it, I never, EVER put any two commands in the same class. Seems kind of crowded you know?

 

I was referring to sub commands.

Like you, I do the same. If I have two different commands, I put them in different classes.

So:
/example <test> AND /example <23> go in the same class, but /example <test> and /something <test> are in separate classes!

 

Sub commands. You mean args? Why not just run those within switch statements?

 

I set the executor for all commands in a CommandHandler class, then have a separate class for every command

https://github.com/gomeow/BukkitSer...rforbukkitpeople/commands/CommandHandler.java

 

Yes, I do that, but some people handle them via separate classes. Look at the thread I posted in the main post.

I mean something like this:

 

Oh, then just create a central command hub that the onEnable() will execute to, and then when an arg is called just run a new instance of the class containing that subcommand. Similar to what gomeow is doing, but much simpler in my head.

 

*facepalm*
I know how to do it ... the question is 'What do YOU, as in plugin developers, do? What method of handling commands do you guys use?'

 

Hey, I'm just here to help. I thought you were asking how to annotate the commands, which I think is just completely unnecessary.

 

It doesn't sound like you were trying to help teach me how to annotate ...

I know how to annotate methods anyway. No need to teach me

 

Well I don't so someone explain what annotating commands is? :3

 

http://docs.oracle.com/javase/tutorial/java/javaOO/annotations.html

 

Right so where do these come into player with commands? I've seen Override and obviously EventHandler annotations around, but how can you run commands with them?

 

If you have a lot of commands, like MobArena by garbagemule does, what you can do is for every class inside a HashMap, get it's annotation, see if the name in the annotation equals the command, then check if the player has the permission in the annotation, and so forth. If everything fits, continue on and run the class which executes the command.

 

Unless the target JVM is version 7 or later, you can't switch on strings. Besides, a massive switch on types that aren't enums is as a rule of thumb as bad as a big if-else tree, which is bad for readability. For command handling, the Command Pattern is very useful. The idea is to encapsulate functionality into objects, and call an execute-method on the objects at the right time.

As number1_Master says, MobArena makes use of the Command Pattern by making a class for each command, and registering (adding to a map) these commands. Simplified, the following piece of pseudocode does what you want to do (assuming no null pointers and perfectly sized arrays - you must check for this of course):

PHP:

// The map containing the commands
private Map<String,Command> commands;
 
// The actual command handler method
public boolean onCommand(Sender sender, String[] args)
  Command cmd = commands.get(args[0]);
  if (sender.hasPermission(cmd.getPermission())) {
    return cmd.execute(sender, args);
  } else {
    // Permission denied code
    return true; // to avoid usage-message
  }
}
 
...
 
// Command interface
public interface Command {
  public boolean execute(Sender sender, String[] args);
}
 
...
 
// Implementing class
public class TeleportCommand implements Command {
  @Override
  public boolean execute(Sender sender, String[] args) {
    // Run command-specific code here
  }
}

The initialization code is up to you. In MobArena I used annotations and reflection for this, but you could easily just do stuff like commands.put("tp", new TeleportCommand()); and repeat for every command.

 

I meant switch statements for args, not the actual commands themselves. That's why I was asking about args.

 

Yeah and as I said, only Java 7+ can switch on Strings. The arguments are in the form of a String array, and are thus Strings, and can thus not be switched upon in a switch-statement in JVMs prior to Java 7. It's possible to indirectly switch on Strings through an Enum wrapper, but that would just add even more maintability issues, and it completely destroys any desires for dynamic arguments. I have a feeling I'm going out on a debate on semantics here though, so I'm sorry if that's the case. At any rate, dynamic, complex content usually calls for more objects in OOP.

I think what the OP is asking for is a general solution to the problem, which would be the Command Pattern. Assuming sub-sub commands or dynamic arguments, other objects could come into play - e.g. joining an arena in MobArena with '/ma join arena1', where 'ma' is the main command, 'join' is the sub command, and 'arena1' is the dynamic argument which is used to fetch an Arena object to call the join() method on. I will go as far as to say that if it's possible to replace branches (if-else trees and switches) with objects (of interface types, hello Strategy Pattern) and method calls, it's almost always worth it from a flexibility and maintainability standpoint, but I'm also extremely pattern horny

 

Don't worry, I'm with you there!

 

garbagemule
I use if else 's on the label and args, and a switch on the args length

 

This is what I do.

 

I don't know exactly what your plugins entail and what kind of commands you're using, but for a completely generalized situation with an arbitrary number of commands, each with arbitrary numbers of arguments of arbitrary data types, I still think the branch-trees are harder to read, and they can be very unforgiving when it comes to maintainability. As an example, MobArena's command handler started out as an if-else tree of maybe 200 lines of code, but grew to over 1100 lines (with over 20 different commands) before I got tired of it and decided it was too much. The idea behind the Command Pattern is to move command-specific logic into a class/object, allowing not only high maintainability, but also an easy way of using the logic in multiple places - digging through 1100 lines of code to find a specific command is (arguably) sub-optimal compared to simply opening a source code file, and if the logic is needed elsewhere (from other classes), copy/pasting the code may be the only option with the mega handler, which is definitely not desirable.

After encapsulating each command, however...

Switching on the length of the arguments array (once the main command has been identified and its execute method called) seems somewhat limiting to me in the general case, but I can think of a situation where it would make more sense than a simple if-else or somehow processing the arguments in (possibly else-less) if-statements, or using the choice operator, along the way; at least with regards to readability. I'd like to delve into an example of parsing items from strings. Let's assume we want the command '/item <item>' to give <item> to the player. The exact format of <item> will be '[ID|name])data):amount)' - looks scary, but it basically means one of three things:

1. Item ID or item name, e.g. '322' or 'golden_apple'
2. Item ID/name, followed by a colon and the amount, e.g. '1:5' or 'stone:5'
3. Item ID/name, followed by a colon and a data value, followed by colon and amount, e.g. '373:16389:1'

This format is intuitively simple: we type the item name or ID followed by a sub type if it has one and an amount if we want more than one of the item. In code, it becomes a bit of a nightmare (swapping amount and data values would make it so much easier, but not as intuitive), but here are three different approaches (just assume that the parse methods exist):

PHP:

// Ifs
Item parseItem1(String s) {
  String[] parts = s.split(":");
  int id = parseID(parts[0]);
  if (parts.length == 1) {
    return Item(id, 0, 1);
  }
  if (args.length == 2) {
    int amount = parseAmount(parts[1]);
    return Item(id, 0, amount);
  }
  byte data = parseData(parts[1]);
  int amount = parseAmount(parts[2]);
  return Item(id, data, amount);
}
 
// Choice
Item parseItem2(String s) {
  String[] parts = s.split(":");
  int id = parseId(parts[0]);
  byte data = (parts.length == 3 ? parseData(parts[1]) : 0);
  if (parts.length == 1) {
    return Item(id, data, 1);
  }
  int amount = parseAmount(parts.length > 2 ? parts[2] : parts[1]);
  return Item(id, data, amount);
}
 
// Switch
Item parseItem3(String s) {
  String[] parts = s.split(":");
  int id = parseID(parts[0]);
  switch (parts.length) {
    case 1:
      return Item(id, 0, 1);
    case 2:
      int amount = parseAmount(parts[1]);
      return Item(id, 0, amount);
    case 3:
      byte data = parseData(parts[1]);
      int amount = parseAmount(parts[2]);
      return Item(id, data, amount);
  }
}

I'm sure we can agree that in this case, switching on the length of the arguments array makes the code somewhat more readable than the ifs and the choice operator (depending on whether or not you like it, I guess). The code is fairly short, and due to the parse methods, not much code is repeated. This is a very specific case, however, and depending on the command, switching may not even be possible - consider a command that takes an arbitrary number of arguments.

At any rate, you'll always want to aim for your code to be "DRY" (of course while ensuring high readability and maintainability), and the branch-trees (be they switch-statements or if-else trees) can be a serious pitfall in that direction, and also outside of command handling.

 

I vote you make a book !

 

Haha! I'll take that as a compliment, but there are already lots of great books out there that explain this stuff in much greater detail, and probably much better than I have here. If you're interested in design patterns, I've heard many great things about the Gang of Four book, but I only read other books that refer to it a lot (in various contexts). I also heard Code Complete is really, really good and easy to understand.

 

I'm all over algorithms, design patterns, and code refactoring !