Java CommandLine Interface implementation

Question

I write a lot of mini apps. Each app has its own InputHandler for input from the keyboard. It's time to make a lib for that purpose - a CommandLineInterface.

NOTE: there exists a follow up Question here

Can you review my code on

• Design
• OOP
• Clean Code
• Naming (my bad englisch leads to bad naming)

Interface CommandLineInterpreter

Any class that implements this interface can use the CommandLineInterface. M is the application, which implements the CommandLineInterpreter

public interface CommandLineInterpreter<M> { Set<Command<M>> getCommands(); Response executeCommand(String identifier, List<String> parameter); } 

Class Command

the Command is responsible to map the command typed via CLI into an action (method-call) on your app. To execute the command the Class is generic on M (which is your app) so you can call these app-specific methods.

public abstract class Command<M> { private final String identifier; public Command(String identifier) { this.identifier = identifier; } public abstract Response execute(M invoked, List<String> parameter); public String getIdentifier() { return identifier; } public boolean isIdentifier(String ident) { return identifier.equals(ident); } @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; Command command = (Command) o; return Objects.equals(identifier, command.identifier); } @Override public int hashCode() { return Objects.hash(identifier); } } 

Class Response

Provides information if the execution of an command was successful or not.

public class Response { private final String message; private final String details; private final Type type; private Response(Type type, String message, String details) { this.type = type; this.message = message; this.details = details; } public static Response fail(String message) { return fail(message, ""); } public static Response fail(String message, String details) { return response(Type.FAILURE, message, details); } private static Response response(Type type, String message, String details) { return new Response(type, message, details); } public static Response success() { return success("ok", ""); } public static Response success(String message) { return success(message, ""); } public static Response success(String message, String details) { return response(Type.SUCCESS, message, details); } public boolean failed() { return type == Type.FAILURE; } @Override public String toString() { return type.toString() + ":" + message + (details.isEmpty() ? "" : " Details: " + details); } private enum Type {SUCCESS, FAILURE} 

Class CommandLineInterface

This class handles the user input from the command line. It is itself a CommandLineInterpreter and offers two basic commands: help and exit.

public class CommandLineInterface implements CommandLineInterpreter<CommandLineInterface> { private static final String COMMAND_SEPARATOR = " "; private final CommandLineInterpreter cli; private final InputStream input; private final PrintStream output; private boolean isRunning = true; public CommandLineInterface(CommandLineInterpreter<?> cli, InputStream input, PrintStream output) { if (cli == null || hasPredefinedCommands(cli.getCommands())) { throw new RuntimeException("CommandLineInterpreter interface of " + cli + " is not properly implemented"); } this.cli = cli; this.input = input; this.output = output; } private boolean hasPredefinedCommands(Set<? extends Command<?>> commands) { return !Collections.disjoint(commands, getCommands()); } public void start() { Scanner scanner = new Scanner(input); showHelp(); while (isRunning) { output.print("$>"); String command = scanner.nextLine(); List<String> line = Arrays.asList(command.split(COMMAND_SEPARATOR)); String identifier = line.get(0); List<String> parameters = line.subList(1, line.size()); if (isExecutableCommand(identifier)) { Response response = executeCommand(identifier, parameters); if (response.failed()) { showResponse(response); } } else { showHelp(); } } } private boolean isExecutableCommand(String identifier) { for (Command cmd : getAllCommands()) { if (cmd.isIdentifier(identifier)) { return true; } } return false; } private void showHelp() { Set<Command<?>> commands = getAllCommands(); output.println("help - these commands are available:"); commands.forEach(c -> output.printf(" - %s\n", c.getIdentifier())); } private void showResponse(Response response) { output.println(response); } @Override public Set<Command<CommandLineInterface>> getCommands() { Set<Command<CommandLineInterface>> cliCommands = new HashSet<>(); cliCommands.add(new Command<CommandLineInterface>("exit") { @Override public Response execute(CommandLineInterface commandLineInterface, List<String> parameter) { isRunning = false; return Response.success(); } }); cliCommands.add(new Command<CommandLineInterface>("help") { @Override public Response execute(CommandLineInterface commandLineInterface, List<String> parameter) { showHelp(); return Response.success(); } }); return cliCommands; } private Set<Command<?>> getAllCommands() { Set<Command<?>> commands = mapCommands(cli.getCommands()); commands.addAll(getCommands()); return commands; } private Set<Command<?>> mapCommands(Set commands) { Set<Command<?>> mappedCommands = new HashSet<>(); for (Object o : commands) mapCommand(o).ifPresent(mappedCommands::add); return mappedCommands; } private Optional<Command<?>> mapCommand(Object o) { return (o instanceof Command<?>) ? Optional.of((Command<?>) o) : Optional.empty(); } @Override public Response executeCommand(String identifier, List<String> parameter) { Optional<Command<CommandLineInterface>> cmd = getCommands().stream().filter(command -> command.isIdentifier(identifier)).findAny(); if (cmd.isPresent()) { return cmd.get().execute(this, parameter); } else { return cli.executeCommand(identifier, parameter); } } } 

Example

If you have implemented the interfaces properly you should have easy to use CLI support for your apps:

public static void main(String[] args) { ExampleApplication app = new ExampleApplication();//implements CommandLineInterpreter CommandLineInterface commandLineInterface = new CommandLineInterface(app, System.in, System.out); commandLineInterface.start(); } 

Output

this is an example output from an example app that does not much...

help - these commands are available: - exampleCommand - help - exit $>help help - these commands are available: - exampleCommand - help - exit $>exampleCommand p1 p2 p3 i could do some actual work now if i were not a mere example, parameter [p1, p2, p3] $>exit Process finished with exit code 0 

Answer 1

Generics

To execute the command the Class is generic on M (which is your app)

public abstract class Command<M> { // .. public abstract Response execute(M invoked, List<String> parameter); // .. 

\$ \$

M is the application, which implements the CommandLineInterpreter

public interface CommandLineInterpreter<M> { Set<Command<M>> getCommands(); Response executeCommand(String identifier, List<String> parameter); } 

Both quotes are wrong, because M would allow every reference type like Integer or CustomClassXY.

To implement it like in the quote described that M can only be CommandLineInterface you do not need generics..

abstract class Command { // .. public abstract Response execute(CommandLineInterface invoked, List<String> parameter); // .. } interface CommandLineInterpreter { Set<Command> getCommands(); Response executeCommand(String identifier, List<String> parameter); } 

After that we can change the redundant methods to map the Commands inside a Set

private Set<Command<?>> getAllCommands() { Set<Command<?>> commands = mapCommands(cli.getCommands()); commands.addAll(getCommands()); return commands; } private Set<Command<?>> mapCommands(Set commands) { Set<Command<?>> mappedCommands = new HashSet<>(); for (Object o : commands) mapCommand(o).ifPresent(mappedCommands::add); return mappedCommands; } private Optional<Command<?>> mapCommand(Object o) { return (o instanceof Command<?>) ? Optional.of((Command<?>) o) : Optional.empty(); } 

Can know look like

private Set<Command> getAllCommands() { Set<Command> commands = cli.getCommands(); commands.addAll(getCommands()); return commands; } 

You can read more about Generics in Effective Java from Joshua Bloch.

A More Suitable Data Structure

// in commandLineInterface.executeCommand Optional<Command<CommandLineInterface>> cmd = getCommands().stream() .filter(command -> command.isIdentifier(identifier)) .findAny(); 

The task of executeCommand is to find only oneCommand from getCommands. The method signature of getCommands is

Set<Command<CommandLineInterface>> getCommands() 

A Set only provides its content through an Iterator, so we have to iterate all elements to get the element we want with a time complexity of \$O(n)\$

A Map in the form of Map<Indentifier, Command> is equivalent to a Set but with the benefit that we can access a element through the get-method in \$O(1)\$.

Optional<Command<CommandLineInterface>> cmd = Optional.ofNullable(getCommands().get(identifier)); 

---

Feature Envy

The whole point of objects is that they are a technique to package data with the processes used on that data. A classic [code] smell is a method that seems more interested in a class other than the one it is in. The most common focus of the envy is the data.

In Method hasPredefinedCommands

When we take a look into the constructor of CommandLineInterface, we can find a method hasPredefinedCommands.

public CommandLineInterface(CommandLineInterpreter<?> cli, InputStream input, PrintStream output) { if (cli == null || hasPredefinedCommands(cli.getCommands())) 

Since the method gets only used at one place and is private we know that the argument commands will always be the commands of a CommandLineInterpreter.

private boolean hasPredefinedCommands(Set<? extends Command<?>> commands) { return !Collections.disjoint(commands, getCommands()); } 

This method should be renamed to contains and moved into CommandLineInterpreter.

public CommandLineInterface(CommandLineInterpreter<?> cli, InputStream input, PrintStream output) { if (cli == null || cli.contains(getCommands()) 

Responsibilities Of CommandLineInterface

You described the CommandLineInterface with

This class handles the user input from the command line. It is itself a CommandLineInterpreter and offers two basic commands: help and exit.

The tagged and in the quote shows that the CommandLineInterface has two responsibilities:

• handles the user input
• offers two basic commands

Choose a Different Name Like CommandLineInteraction

The name CommandLineInterface is so abstract, that it could be everything and from a programmer perspective I would thing that this is an interface and not a class.. Maybe the responsibilities for this class makes more sense, if we change the name to ComandLineInteraction.

With this name I associate thinks the already existing method start and but also thinks like stop, clean, isRunning and so on..

Let's Remove the Second Responsibility

First we need to check the reason, why it currently is in CommandLineInterface. In the method CommandLineInterface.getCommands:

public Set<Command<CommandLineInterface>> getCommands() { // .. cliCommands.add(new Command<CommandLineInterface>("exit") { @Override public Response execute(CommandLineInterface commandLineInterface, >List<String> parameter) { isRunning = false; return Response.success(); } }); // .. } 

The reason is the statement isRunning = false, because isRunning is a private variable of CommandLineInterface and you can only access it inside it.

You already provide a method start and we could create a method stop too. After that the creation of the commands does no longer need to be in CommandLineInterface.

The Method stop

public void stop() { isRunning = false; } 

The method showHelp should be public

Inject Default Commands through the Contractor

public CommandLineInteractions(..., Map<String, Command> default) { // ... this.commands = default; } 

From the outside it now looks like

Map<String, Command<>> defaults = new HashMap<>(); defaults.put("exit", new Command("exit") { @Override public Response execute(CommandLineInterface commandLineInterface, List<String> parameter) { commandLineInterface.stop(); return commandLineInterface.isRunning() ? Response.fail("App is still running") : Response.success(); } }); CommandLineInterface app = new Command(..., defaults); 

---

To The Comments

ok about the proper data type (map instead of set) - i want to free the app of creating a redundant data type (identifier, Command) since the command already provides a method getIdentifier()... so any CommandLineInterpreter has to only return the commands that are availible. the amount of commands is rather short, i don't expect more than a dozends commands to be availible so i tend to CleanCode rule don't optimze , see https://clean-code-developer.com/grades/grade-1-red/#Beware_of_Optimizations

From the link you provide:

Understandability of code hast always priority. Optimized code pretty often is all but well readable.

I think the optimization rule doesn't apply to change the Set to an Map, since the Map is the more readable and intuitive way of getting a Command by its name.

i want to free the app of creating a redundant data type (identifier, Command) since the command already provides a method getIdentifier()...

The identifier can still be the string you used before. So it is not really redundant.

When we philosophize a little bit: Does a command need an identifier (name) by it self?

The Map-solution would make the identifier in the command useless and more flexable:
Imagine you use your tool on work and you have a new Spanish employee with low (in your case German) or English skills. Your tool currently only works in german/english and the new employee don't know how to use it.
With Set solution you need to make a new Command, witch is doing the same only with a new identifier, but with the Map solution you put in the reference of the same command but with a different key.

Create a First Class Collection

i want to free the app

What is a First Class Collection:

Any class that contains a collection should contain no other member variables. Each collection gets wrapped in its own class, so now behaviors related to the collection have a home.

When you know want to switch from a Set to a Map or what ever data-type you need to change at sooo many places. But with a FCC you need only to change inside the FCC.

The FCC would define all the methods you need like get and contains and the outer world doesn't care if you are using inside it a Set or a Map - or short : data encapsulation.