1

this feels like I am cheating or doing something wrong. I am a Java student working on a simple JavaFX project.

As I loop through and create buttons in a flowPane, I was having trouble using the loop counter i inside an inner class. It's the part where I assign event handlers. I have dealt with this issue before, I get the difference between "final" and "effectively final" so I don't believe I am asking that.

It's just that creating this copy of i by using "int thisI = i" just feels wrong, design-wise. Is there not a better way to do this? I looked into lambdas and they also have the "final or effectively final" requirement.

Here's my code, any level or criticism or suggestion for improvement is welcome, thanks!

private FlowPane addFlowPaneCenter() {

    FlowPane flow = new FlowPane();
    flow.setPadding(new Insets(0, 0, 0, 0));
    flow.setVgap(0);
    flow.setHgap(0);
    flow.setPrefWrapLength(WIDTH_OF_CENTER); // width of function buttons

    Button centerButtons[] = new Button[NUM_BUTTONS];
    ImageView centerImages[] = new ImageView[NUM_BUTTONS];
    for (int i=0; i < NUM_BUTTONS; i++) {
        centerImages[i] = new ImageView(
                new Image(Calculator.class.getResourceAsStream(
                "images/button-"+(i)+".png")));
        centerButtons[i] = new Button();
        centerButtons[i].setGraphic(centerImages[i]);
        centerButtons[i].setPadding(Insets.EMPTY);
        centerButtons[i].setId("button-"+(i));
        flow.getChildren().add(centerButtons[i]);

        // add a drop shadow on mouseenter
        DropShadow shadow = new DropShadow();

        // ***** here's the workaround is this really a good approach
        // to use this in the inner class instead of i? thanks *****
        int thisI = i;

        // set event handlers for click, mousein, mouseout
        centerButtons[i].setOnAction(new EventHandler<ActionEvent>() {
                @Override public void handle(ActionEvent e) {
                    // change graphic of button to down graphic
                    ImageView downImage = new ImageView(new 
                    Image(Calculator.class.getResourceAsStream(
                    "images/button-"+(thisI)+"D.png")));

                    // call function to effect button press
                    System.out.println("Button click");

                    // change graphic back
                    centerButtons[thisI].setGraphic(centerImages[thisI]);

                }});

        centerButtons[i].addEventHandler(MouseEvent.MOUSE_ENTERED, 
                new EventHandler<MouseEvent>() {
                        @Override public void handle(MouseEvent e) {
                        centerButtons[thisI].setEffect(shadow);
                        }
                    });

        centerButtons[i].addEventHandler(MouseEvent.MOUSE_EXITED, 
                new EventHandler<MouseEvent>() {
                        @Override public void handle(MouseEvent e) {
                        centerButtons[thisI].setEffect(null);
                        }
                    }); 
    }
    return flow;
}
JimLohse
  • 1,209
  • 4
  • 19
  • 44

1 Answers1

2

You can remove the arrays centerButtons and centerImages completely. Instead create local variables for the image and the button within the loop and use those, e.g.

final ImageView image = new ImageView(...);
final Button button = new Button();
button.setGraphic(centerImages[i]);
...

You can use the local variables in your eventhandlers, e.g.

button.setOnAction(new EventHandler<ActionEvent>() {
            @Override public void handle(ActionEvent e) {
                ...
                // change graphic back
                button.setGraphic(image);

            }});

Two minor improvements I noticed:

  • Try to avoid creating an Image more than once, because every time you create an Image, the actual data will be loaded again. Your handler will create a new Image for each click. I usually create Images in static final fields.
  • Event handlers are a nice opportunity to practice lambda expressions. :)
  • 1
    In general good advice, but due to quirks in the JavaFX runtime, creating images in static final fields does not work in later JavaFX versions, see [RT-30796 Cannot create a JavaFX Image until "Internal graphics" are initialized](https://javafx-jira.kenai.com/browse/RT-30796) and [How to load image from static method](http://stackoverflow.com/questions/17454593/how-to-load-image-from-static-method) – jewelsea Dec 10 '14 at 22:52
  • I will go ahead and credit this as the answer, after all it's the only answer after several days and it is helpful. But if anyone still wants to answer about whether this way of dealing with "final or effectively final" is valid or has pitfalls, please do so. – JimLohse Dec 15 '14 at 14:35