HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Console component in JavaFX

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
componentconsolejavafx

Problem

I would like a review on my Console class I made in JavaFX 8. It features the following:

  • A text field where you can enter input, the input gets copied into the text area.



  • A text area where you can observe output and see the input.



  • When you press enter, you send the message.



  • When you press arrow up, you can go back in history.



  • When you press arrow down, you can go forward in history.



  • You can hook an event listener when a message gets sent.



```
public class Console extends BorderPane {
protected final TextArea textArea = new TextArea();
protected final TextField textField = new TextField();

protected final List history = new ArrayList<>();
protected int historyPointer = 0;

private Consumer onMessageReceivedHandler;

public Console() {
textArea.setEditable(false);
setCenter(textArea);

textField.addEventHandler(KeyEvent.KEY_RELEASED, keyEvent -> {
switch (keyEvent.getCode()) {
case ENTER:
String text = textField.getText();
textArea.appendText(text + System.lineSeparator());
history.add(text);
historyPointer++;
if (onMessageReceivedHandler != null) {
onMessageReceivedHandler.accept(text);
}
textField.clear();
break;
case UP:
if (historyPointer == 0) {
break;
}
historyPointer--;
runSafe(() -> {
textField.setText(history.get(historyPointer));
textField.selectAll();
});
break;
case DOWN:
if (historyPointer == history.size() - 1) {
break;
}
historyPointer++;
runSafe(() -> {

Solution

As someone who's been working with your project and your Console Gui here's my 2 cents:

protected final TextArea textArea = new TextArea();
protected final TextField textField = new TextField();


these could use a rename. The names do not contain any information and in all the unit tests you wrote, this was confusing me very much: Which is in, which is out?

I'd rather have something like:

protected final TextArea output = new TextArea();
protected final TextField input = new TextField();


textField.addEventHandler(KeyEvent.KEY_RELEASED, keyEvent -> {
    //long block of code...


I like very much that you are using a comprehensively wired lambda. I don't like that you are mixing abstraction levels here. on the one hand, you have that "I configure my console" code like textArea.setEditable(false), and then you have that switch (keyEvent.getCode())

Personally I'd have instead extracted the handler code to a separate handler (which needs a better name than what I am about to propose):

private final EventHandler keyReleasedHandler = new EventHandler<>() {
        handle(final KeyEvent keyEvent){
            //your code here
        }
    }
textField.setOnKeyReleased(keyReleasedHandler);


This also makes it easier to understand, where your constructor ends, without seeing the method signature that follows. While we're at it:

setCenter(textArea);

    //here comes the whole event handling

    setBottom(textField);


Why not put together what belongs together. Both of these are layouting commands, the other two are behavior commands. I'd go for:

setCenter(output);
setBottom(input);

output.setEditable(false);
input.setOnKeyReleased(keyReleasedHandler);


Other than that, your code is very concise and understandable. I like your defensive programming style ;)

Code Snippets

protected final TextArea textArea = new TextArea();
protected final TextField textField = new TextField();
protected final TextArea output = new TextArea();
protected final TextField input = new TextField();
textField.addEventHandler(KeyEvent.KEY_RELEASED, keyEvent -> {
    //long block of code...
private final EventHandler<KeyEvent> keyReleasedHandler = new EventHandler<>() {
        handle(final KeyEvent keyEvent){
            //your code here
        }
    }
textField.setOnKeyReleased(keyReleasedHandler);
setCenter(textArea);

    //here comes the whole event handling

    setBottom(textField);

Context

StackExchange Code Review Q#52197, answer score: 8

Revisions (0)

No revisions yet.