BRIEF EXPLANATION.
I am implementing a very simple command parser that requires me to handle a few commands:
pw path
, mv source destination
, help
, and some others.
The commands always start with the command name
, and then there are either 0, 1 or 2 strings after that. I do not exclude that the number of strings may change in the future, but the structure is always the same (as it is now).
I thought I could use the command pattern for this. Everything seems good in my schema. However, when I attempted to implement it, I realized that there's a flaw.
I currently pass the necessary information to the constructors of each Command (each command has its own class as the design pattern suggests). For example, the CdCommand
constructor takes the FileSystem and a String Path
, the MvCommand
takes the FileSystem and String source, String dest
, while the HelpCommand
only takes the FileSystem.
The issue is that I only know the inputs after I have called the constructors. That's because in my design I initialize the HashMap inside CommandExecutionController
at the beginning of the program, by hardcoding the command names and passing in the correct command objects.
I could solve this by mandating that all Command classes have a String...
parameter in their execute()
method. But this would also force commands that don't require any string (such as the help
command) to have this unneeded parameter.
I am looking for ways to mitigate this issue in practice. I've seen multiple times that the Command Pattern might be a fair solution for this, but haven't yet found any thread that explains what exactly could be done to solve the issue I'm experiencing.
Ideally I would want to follow this pattern and avoid if-else
or switches
(because they're not scalable), and especially avoid the String...
parameter (because not all concrete command classes require it).
ACTUAL CODE.
CommandExecutionController
:
public class CommandExecutionController {
private final HashMap<String, Class<? extends CommandInterface>> commandList;
private final FileSystemModel fileSystemModel;
public CommandExecutionController(final FileSystemModel fileSystemModel) {
this.commandList = new HashMap<>();
this.fileSystemModel = fileSystemModel;
}
public void register(final String commandName, Class<? extends CommandInterface> commandClass) {
commandList.put(commandName, commandClass);
}
public CommandInterface getDispatchedCommand(final String commandName, final String... arguments)
throws NoSuchMethodException, InvocationTargetException, InstantiationException, IllegalAccessException
{
if (commandList.containsKey(commandName)) {
Class<? extends CommandInterface> command = commandList.get(commandName);
return command.getConstructor(FileSystemModel.class, String[].class).newInstance(fileSystemModel, arguments);
}
throw new IllegalArgumentException("Command NOT found: " + commandName);
}
CdCommand
:
public class CdCommand implements CommandInterface {
private final String path;
private final FileSystemModel fileSystemModel;
public CdCommand(final FileSystemModel fileSystemModel, final String... arguments) {
this.path = arguments[0];
this.fileSystemModel = fileSystemModel;
}
@Override
public String execute() {
return fileSystemModel.cd(path);
}
}
How I'm currently calling it:
text = text.trim();
final String[] commandParts = text.split("\\s+");
final boolean isCommandCorrect = commandParts.length > 0 && checkCommandExistence(commandParts[0]);
// Test dispatching
if (commandParts.length > 0) {
try {
final CommandInterface command = commandExecutionController.getDispatchedCommand(commandParts[0],
String.join(" ", Arrays.copyOfRange(commandParts, 1, commandParts.length))
);
return command.execute();
} catch (NoSuchMethodException | InvocationTargetException | InstantiationException |
IllegalAccessException e) {
throw new RuntimeException(e);
}
}
I've looked up other stack overflow threads regarding this question and the command pattern, but found no solutions for my current issue.
I am replying to my own thread since I found a "workaround".
First of all, I created a CommandsInfo annotation:
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
public @interface CommandInfo {
String name();
int totalArguments();
}
Next, I make sure that all my command classes are annotated with it. For example for Cd
:
@CommandInfo(name = "cd", totalArguments = 1)
public class CdCommand implements CommandInterface {
Now, I use the Reflections
library. This lets me avoid having to register all the commands manually. Since I have all the necessary classes annotated, I just scan in the current Commands package, find all classes that implement the CommandInterface and are annotated with the CommandInfo
. By doing this I can insert all the command classes with their relative command name in my HashMap. The idea is that all I need to add a new command is creating its class and annotate it.
private void initializeAllCommands() {
// Self notes
// 1. Scan the current package to find all the classes that implement the CommandInterface AND are annotated with CommandInfo.
final Package currentPackage = CommandExecutionController.class.getPackage();
final Reflections reflections = new Reflections(currentPackage.getName());
final Set<Class<?>> annotatedClasses = reflections.getTypesAnnotatedWith(CommandInfo.class);
for (Class<?> annotatedClass : annotatedClasses) {
// 2. Check if the current class implements the CommandInterface.
if (CommandInterface.class.isAssignableFrom(annotatedClass)) {
// 3. Check if there is the CommandInfo annotation for the current command class.
final CommandInfo annotation = annotatedClass.getAnnotation(CommandInfo.class);
if (annotation != null) {
commandList.put(annotation.name(), annotatedClass.asSubclass(CommandInterface.class));
}
}
}
}
Next, I call the dispatcher by passing the command name retrieved from the user input and the rest of the arguments.
try {
final CommandInterface command = commandExecutionController.getDispatchedCommand(commandParts[0],
Arrays.copyOfRange(commandParts, 1, commandParts.length));
return command.execute();
}
Next, all we need is to verify the syntax of the command. This becomes easier because all the classes are annotated with command name + the number of arguments that it takes:
if (commandList.containsKey(commandName)) {
final Class<? extends CommandInterface> genericCommand = commandList.get(commandName);
final CommandInfo annotation = genericCommand.getAnnotation(CommandInfo.class);
if (annotation.totalArguments() != arguments.length) {
throw new IllegalArgumentException("Wrong number of arguments for command: " + commandName
+ " [Expected: " + annotation.totalArguments() + ", provided: " + arguments.length + "]");
}
return genericCommand.getConstructor(FileSystemModel.class, String[].class)
.newInstance(fileSystemModel, arguments);
And finally, each command constructor can still keep the String...
parameter. Since at the point of constructor we know that the syntax is correct, we can just do something like this as an example:
public MvCommand(final FileSystemModel receiver, final String... arguments) {
this.receiver = receiver;
this.origin = arguments[0];
this.destination = arguments[1];
}
If we have a command that requires no argument, we can just avoid the String...
parameter whatsoever.
This seems to work fine. It might be overengineered, but hey...
(I would also like to thank vanOekel for their answer, by the way. I've read it too late!!)