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
StringBuilderinstead of aStringBuffer.StringBuilderis 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
stringBufferinside thewhileloop. 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),stringBufferis 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 twoStringobjects, one of which is immediately discarded after the line and only serves as an intermittentStringto calltrim()on, while the second line only creates oneStringobject, that which is stored inbufferContent. Note that the second line will also modifystringBuffer, while the first line does not. However, sincestringBufferis 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
whileloop's control flow is easier to infer from the code structure inside it alone, because you have onecontinuestatement less.Exterminate duplicate code in the two
ifclauses by filtering out the common behavior. The only differences are that, in the secondifclause, the last element ofbufferCommandsis not iterated over but added tolineListafterlineListis cleared. A way to represent this logic more concisely might be replacing the twoifclauses with one largerifclause: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]); } }