Editing Javascript and rendering HTML in one window v2

Posted on

Problem

I wanted to revisit this site to post some recent work I put into this small project. I was able to add a console as suggested; but I thought I would ask here how my coding stying is before I work on it more to hopefully avoid getting too frustrated with modifying what may be bad coding style. How has this improved? How has it not improved?

import java.util.ArrayList;
import javafx.application.Application;
import javafx.beans.value.ChangeListener;
import javafx.beans.value.ObservableValue;
import javafx.geometry.Orientation;
import javafx.scene.Scene;
import javafx.scene.control.SplitPane;
import javafx.scene.control.TextArea;
import javafx.scene.input.KeyEvent;
import javafx.scene.paint.Color;
import javafx.scene.web.WebEngine;
import javafx.scene.web.WebView;
import javafx.stage.Stage;
import netscape.javascript.JSException;
import netscape.javascript.JSObject;

public class Jjs extends Application {

    // source HTML view
    private TextArea source;
    // WebView render of source HTML "console" variable
    private WebView webView;
    // console view helped by separate class
    private JsConsole console;

    // sets up main view and fills with red to make formatting errors obvious
    @Override
    public void start(Stage primaryStage) {
        SplitPane splitPane = new SplitPane();
        splitPane.getItems().addAll(leftPane(), rightPane());Scene scene = new Scene(splitPane);
        scene.setFill(Color.RED);
        primaryStage.setScene(scene);
        primaryStage.show();
    }

    // sets up HTML source view and adds text listener
    // the render view updates with every single edit: an error resets
    // the rendering view, a simple erraneous input such as an extra
    // character causes a refresh
    private SplitPane leftPane() {
        source = new TextArea();
        source.setStyle("-fx-font-family: monospace");
        source.textProperty().addListener(new ChangeListener<String>() {
            @Override
            public void changed(final ObservableValue<? extends String> observable,
                    final String oldValue, final String newValue) {
                try {
                    // restores console hooks in web engine after crash
                    console.setConsole(webView.getEngine());
                    webView.getEngine().executeScript(source.getText());
                } catch (JSException e) {
                    webView.getEngine().loadContent(e.toString());
                }
            }
        });
        SplitPane result = new SplitPane();
        result.setOrientation(Orientation.VERTICAL);
        result.getItems().addAll(source);
        return result;
    }

    // sets up the HTML render view and the console view using a helper class
    private SplitPane rightPane() {
        webView = new WebView();
        webView.getEngine().setJavaScriptEnabled(true);
        console = new JsConsole(webView.getEngine());
        SplitPane result = new SplitPane();
        result.setOrientation(Orientation.VERTICAL);
        result.getItems().addAll(webView, console.getTextArea());
        return result;
    }

    // main class - note netbeans does not use this
    public static void main(String[] args) {
        launch(args);
    }
}

class JsConsole {
    // prompt shown at upon line of input availablity
    private final String PROMPT = "> ";
    // -
    private final TextArea textArea = new TextArea();
    // array to hold historical output and calls to "console.log"
    private ArrayList<String> history = new ArrayList<>();
    // input buffer
    private StringBuilder input = new StringBuilder();
    // sets up initial view and adds key listener to capture and absorb
    // keyboard input; keeping the formatting correct
    // attempts to execute every line of input and outputs an error if necessary
    public JsConsole(WebEngine engine) {
        setConsole(engine);
        textArea.setStyle("-fx-font-family: monospace");
        textArea.setOnKeyTyped((KeyEvent ke) -> {
            String ch = ke.getCharacter();
            ke.consume();
            switch (ch.getBytes()[0]) {
                // enter key - process line
                case 13:
                    history.add(PROMPT + input.toString());
                    try {
                        engine.executeScript(input.toString());
                    } catch (JSException jse) {
                        history.add(jse.toString());
                    }
                    input = new StringBuilder();
                    break;
                // backspace key - delete last character
                case 8:
                    input.setLength(input.length() - 1);
                    break;
                // put everything else into the buffer
                default:
                    input.append(ch);
                    break;
            }
            // re-render the console view
            render();
        });
        // calls render instead of printing PROMPT for clean-ness
        render();
    }
    // restores console hooks in web engine after crash
    public void setConsole(WebEngine engine) {
        JSObject window = (JSObject) engine.executeScript("window");
        window.setMember("console", new Console());
    }
    // getter for private variable
    public TextArea getTextArea() {
        return textArea;
    }
    // prints historical items, then prints current line of input
    private void render() {
        textArea.setText("");
        history.stream().forEach((entry) -> {
            textArea.appendText(entry + "n");
        });
        textArea.appendText(PROMPT);
        textArea.appendText(input.toString());
        textArea.positionCaret(textArea.getLength());
    }
    // output class handed off to web view engine
    public class Console {
        public String log(String x) {
            history.add(x);
            render();
            return x;
        }
    }
}

screenshot

Solution

history

history is declared as ArrayList,
it would have been better as a List.

But I’m wondering if it really needs to be a list at all.
How about a StringBuilder?
Appending PROMPT + input.toString() will become history.append(PROMPT).append(input),
which is simpler and efficient.
The rendering method will also become simpler,
as you won’t need to iterate over list elements,
just simply dump text that’s ready for display.

Magic numbers

// enter key - process line
case 13:
    // ...
// backspace key - delete last character
case 8:
    // ...

The comments do a good job there, but it would be even better to just move these values to constants.

Pointless comments

There are too many unnecessary comments.
For example:

// getter for private variable
public TextArea getTextArea() {
    return textArea;
}

There are many more like that, and it makes the code look noisy.

Too dense formatting

The code could use a lot more vertical spacing (blank lines).

Some notes:

  1. What does Jjs mean? I can’t suggest a name, because I can’t understand it.

  2. All your // comments can either become JavaDoc comments, or removed. I suggest JavaDoc comments, which look like:

    /**
     * Comment text here
     */
    
  3. Code here:

    public class Console {
        public String log(String x) {
            history.add(x);
            render();
            return x;
        }
    }
    

    Two things:

    1. As @janos notes, more spaces.
    2. Why is it a public class? Since the class is not used outside of your program, make it either private or default level.

Leave a Reply

Your email address will not be published. Required fields are marked *