Until now, all my command readers expect the entire command on a single line, so that it may be read with the call scanner.nextLine()
. This code here caches the input line and after each Enter attempts to build command(s) from the currently stored lines. If no commands may be built, the previous lines remain in the line list, and the entire program expects more input.
Here is what I have:
import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;
public class CommandLineReader {
private static final String COMMAND_SEPARATOR = ";";
private static final String QUIT_COMMAND = "quit";
private static final String PROMPT = "> ";
public static void main(String[] args) {
Scanner scanner = new Scanner(System.in);
StringBuffer stringBuffer = new StringBuffer();
List<String> lineList = new ArrayList<>();
main_loop:
while (true) {
System.out.print(PROMPT);
String inputLine = scanner.nextLine();
lineList.add(inputLine);
stringBuffer.delete(0, stringBuffer.length());
for (String line : lineList) {
stringBuffer.append(line.trim()).append(' ');
}
String bufferContent = stringBuffer.toString().trim();
String[] bufferCommands = bufferContent.split(COMMAND_SEPARATOR);
boolean lastCommandTerminated =
bufferContent.endsWith(COMMAND_SEPARATOR);
if (!lastCommandTerminated && bufferCommands.length < 2) {
continue;
}
if (lastCommandTerminated) {
for (String bufferCommand : bufferCommands) {
String command = bufferCommand.trim();
System.out.println(command + COMMAND_SEPARATOR);
if (command.equals(QUIT_COMMAND)) {
break main_loop;
}
}
lineList.clear();
} else {
for (int i = 0; i < bufferCommands.length - 1; ++i) {
String command = bufferCommands[i].trim();
System.out.println(command + COMMAND_SEPARATOR);
if (command.equals(QUIT_COMMAND)) {
break main_loop;
}
}
lineList.clear();
// Return the last unfinished command:
lineList.add(bufferCommands[bufferCommands.length - 1]);
}
}
System.out.println("Bye!");
scanner.close();
}
}
A possible session might look like this:
> yargh> aye> captain; abc; def; hello yargh aye captain; abc; def;> world> ; quit; hello world; quit; Bye!
My primary critique concern is usability: Can it be improved?
1 Answer 1
Here are some suggestions for improving the code:
- Use a
StringBuilder
instead of aStringBuffer
.StringBuilder
is equivalent toStringBuffer
, with the exception that it is not safe for multithreading, which makes it a bit faster. In your program, no other thread can ever have access tostringBuffer
, because it is a local variable and you never pass it to a method or constructor, so you don't need synchronization on it. - Declare
stringBuffer
inside thewhile
loop. It is not needed outside it and reset at the beginning of every loop iteration. So declaring it inside the loop reduces its scope to the smallest extent necessary, which makes the code clearer. Replace this line:
String bufferContent = stringBuffer.toString().trim();
with this line:
String bufferContent = stringBuffer.deleteCharAt(stringBuffer.length() - 1).toString();
Since
for (String line : lineList)
will execute at least once (you manually added a line tolineList
, so you can be sure it contains at least one element),stringBuffer
is guaranteed to end with a single space character and have no whitespace directly preceding it, so the purpose oftrim()
is essentially to remove this single space character. The first line above, however, creates twoString
objects, one of which is immediately discarded after the line and only serves as an intermittentString
to calltrim()
on, while the second line only creates oneString
object, that which is stored inbufferContent
. Note that the second line will also modifystringBuffer
, while the first line does not. However, sincestringBuffer
is never accessed again afterwards, this does not matter.In fact, an even more efficient approach would be to never let the final space character be appended in the first place by modifying the
for (String line : lineList)
loop to something like this:for (Iterator<String> lineListIterator = lineList.iterator(); lineListIterator.hasNext(); ) { stringBuffer.append(lineListIterator.next().trim()); if (lineListIterator.hasNext()) { stringBuffer.append(' '); } }
Then you can just call:
String bufferContent = stringBuffer.toString();
This might be more verbose than your original loop combined with the subsequent removal of the space character, but it is also more explicit, and thus maybe easier to read despite the increased verbosity.
Replace this construct:
while (true) { // ... if (!lastCommandTerminated && bufferCommands.length < 2) { continue; } if (lastCommandTerminated) { // ... } else { // ... } }
with this construct:
while (true) { // ... if (lastCommandTerminated) { // ... } else if (bufferCommands.length >= 2) { // ... } }
That way, you convey the exact same message, but with less code, and also, the
while
loop's control flow is easier to infer from the code structure inside it alone, because you have onecontinue
statement less.Exterminate duplicate code in the two
if
clauses by filtering out the common behavior. The only differences are that, in the secondif
clause, the last element ofbufferCommands
is not iterated over but added tolineList
afterlineList
is cleared. A way to represent this logic more concisely might be replacing the twoif
clauses with one largerif
clause:if (lastCommandTerminated || bufferCommands.length >= 2) { for (int i = 0; i < bufferCommands.length - (lastCommandTerminated ? 0 : 1); ++i) { String command = bufferCommands[i].trim(); System.out.println(command + COMMAND_SEPARATOR); if (command.equals(QUIT_COMMAND)) { break main_loop; } } lineList.clear(); if (!lastCommandTerminated) { lineList.add(bufferCommands[bufferCommands.length - 1]); } }