patternjavaMinor
Java CLI Program Commands
Viewed 0 times
commandsprogramclijava
Problem
As I was creating a CLI program, I realized having a ton of switch/if-then statements was a messy way to process user input. So, I set up a way to dynamically create commands as I added modules (Classes that extend the Module class) and methods. When a user enters a command like "sys::get bios" it gets passed to the loaded Module with the ID "sys". The base functionality defined in Module goes through each method in the runtime class and selects the best matching method. In this case, it would find
Process Input Method:
Module run method (Not overridden by a Module Class):
```
public void run(String input)
{
List args = new ArrayList();
Matcher m = argPattern.matcher(input);
while (m.find()) args.add(m.group(1).replace("\"", "")); //Get all args and remove any surrounding quotes
Method method = matchingMethod(args);
if (method == null) //No matching valid command methods
{
Logger.logVerbose("Command no
get_bios() and call it via reflection. I was wondering if anyone else has a way a improve this code, or expand its functionality.Process Input Method:
public static void processInput(String input, boolean recordLast)
{
if (input.startsWith("|"))
{
if (input.length() > 1) {
List output = Runner.runSimple("cmd.exe /c " + input.substring(1, input.length()));
output.forEach(l -> Logger.logVerbose(l));
}
}
else if (input.contains("::"))
{
//Command Syntax: MODULE::COMMAND ARGS
if (ModuleRegistry.getModule(input.split(Pattern.quote("::"))[0]) != null)
{
ModuleRegistry.getModule(input.split(Pattern.quote("::"))[0]).run(input.substring(input.indexOf("::")+2));
if (recordLast) lastInput = input;
}
else
{
Logger.logError("Invalid context: " + input.split(Pattern.quote("::"))[0]);
}
}
else if (input.equals("!"))
{
if (!lastInput.equals("!")) processInput(lastInput);
}
}Module run method (Not overridden by a Module Class):
```
public void run(String input)
{
List args = new ArrayList();
Matcher m = argPattern.matcher(input);
while (m.find()) args.add(m.group(1).replace("\"", "")); //Get all args and remove any surrounding quotes
Method method = matchingMethod(args);
if (method == null) //No matching valid command methods
{
Logger.logVerbose("Command no
Solution
I've really only taken a deep look at the
-
If this function is invoked with just
-
The last
-
Split this up; it's too long and makes your code harder to follow and more error-prone:
-
Remove the duplicate statement to make this method neater:
-
Lots of static methods. Consider a more object-oriented approach.
I haven't looked too in depth at the reflection logic, but two comments after a high level look:
-
Wouldn't this be better as a Bash/Python script? Dispatching methods dynamically through Java reflection for general operating system specific tasks strikes me as the wrong tool for the job. Seems error prone, a security risk and an unnecessary level of indirection.
-
In order to expose a class to your framework you require it to be a subclass of module, effectively tying it to this type hierarchy. If you do keep this design, consider making run
processInput method. Still, some comments:-
If this function is invoked with just
"::" as an input argument it will throw an ArrayIndexOutOfBoundsException exception.-
The last
if statement looks redundant. lastInput seems as if it can only be set in the else if (input.contains("::")) block, so can never be "!".-
Split this up; it's too long and makes your code harder to follow and more error-prone:
ModuleRegistry.getModule(input.split(Pattern.quote("::"))[0]).run(input.substring(input.indexOf("::")+2));-
Remove the duplicate statement to make this method neater:
ModuleRegistry.getModule(input.split(Pattern.quote("::"))[0]-
Lots of static methods. Consider a more object-oriented approach.
I haven't looked too in depth at the reflection logic, but two comments after a high level look:
-
Wouldn't this be better as a Bash/Python script? Dispatching methods dynamically through Java reflection for general operating system specific tasks strikes me as the wrong tool for the job. Seems error prone, a security risk and an unnecessary level of indirection.
-
In order to expose a class to your framework you require it to be a subclass of module, effectively tying it to this type hierarchy. If you do keep this design, consider making run
final as there's nothing to stop clients of your framework overriding this method.Code Snippets
ModuleRegistry.getModule(input.split(Pattern.quote("::"))[0]).run(input.substring(input.indexOf("::")+2));ModuleRegistry.getModule(input.split(Pattern.quote("::"))[0]Context
StackExchange Code Review Q#140513, answer score: 3
Revisions (0)
No revisions yet.