Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TableView setCellValueFactory() goes crazy with EventStream binding #22

Closed
thomasnield opened this issue Jun 19, 2015 · 8 comments
Closed

Comments

@thomasnield
Copy link

Hey guys, I have found a problem in using reactive programming with JavaFX. The TableView TableColumn setCellValueFactory does not like being mapped to an EventStream binding. I noticed I can get an Observable in RxJava to work sometimes, but the moment any threads, schedulers, RxJava to ReactFX conversions, or Platform.runLater() calls are introduced chaos ensues. There is no way to effectively bring the subscription to the JavaFX thread without more complications. In many of these scenarios, the TableView keeps calling the cellValueFactory infinitely and re-emitting the values each time, but the values never make it to the table. Other times, no emissions make it at all.

I've spammed SO with a few questions on the odd reactive behaviors I've been finding with TableView. TableColumn setCellValueFactory. Is there an easy solution for this?

http://stackoverflow.com/questions/30804401/javafx-and-rxjava-tableview-infinitely-calling-setcellvaluefactory

http://stackoverflow.com/questions/30931384/rxjava-javafx-property

@TomasMikula
Copy link
Owner

Hi Thomas,

the questions are beyond my knowledge of RxJava and TableView. I was going to suggest to try to pinpoint the problem by trying to eliminate ReactFX or RxJava out of the example, but I see you already did eliminate ReactFX in you latter question.

Btw, where does the Observable in your real application come from? In your example, you use Observable.just(10,20,30,40,50,60), but where would that observable come from in a real application?

You can try replacing it with a ReactFX EventStream and see if that helps. In ReactFX, all streams are potentially infinite, but you can subscribe to just a certain number of events. You can mimick the above RxJava Observable like this:

EventStreams.ticks(Duration.ofSeconds(2)) // emits a value every 2 seconds
        .accumulate(0, (last, tick) -> last + 10)
        .subscribeFor(6, v -> { /* event handler */ });

@thomasnield
Copy link
Author

I guess with my production code, the Observable would come from a Subject emitting a Map on every data refresh... or a business algorithm composed from several iterable Observables to come up with a single, completed, emitted result.

@thomasnield
Copy link
Author

And my production code at work worked just fine with the RxJava updating the Property. But I guess it didn't work with an impractical example like this... I will need to do some more testing and managing my expectations. I guess I was looking for an across-the-board solution regardless of the emission scheduled behavior. I do want to confirm the pure EventStream behavior in a TableView cellValueFactory though.

@thomasnield
Copy link
Author

There is still something odd when I use just the EventStream in a JavaFX TableView. It has the same problem. The values never reach the binding and never display in the TableView. The values for the single record stay at the initial binding value of -1. Try running this and you will see, and pay attention to the console as it prints when an emission happens.

public class ReactiveTableViewTest extends Application {

    @Override
    public void start(Stage stage) throws Exception {

        Group root = new Group();
        Scene scene = new Scene(root);

        root.getChildren().add(new ReactiveTable(buildSampleData()));

        stage.setScene(scene);
        stage.show();
    }

    private ObservableList<ReactivePoint> buildSampleData() { 

        EventSource<Integer> evt1 = new EventSource<>();
        EventSource<Integer> evt2 = new EventSource<>();

        Executor exec = Executors.newFixedThreadPool(5);

        TriFunction<EventSource<Integer>, Integer, Integer, Runnable> toRunnable = (s,i,t) -> {
            return new Runnable() { 
                public void run() { 
                    try {
                        Thread.sleep(t);
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                    s.push(i);
                }
            };
        };
        exec.execute(toRunnable.apply(evt1, 5, 3000));
        exec.execute(toRunnable.apply(evt2, 3, 3000));
        exec.execute(toRunnable.apply(evt1, 10, 6000));
        exec.execute(toRunnable.apply(evt2, 6, 6000));
        exec.execute(toRunnable.apply(evt1, 15, 9000));
        exec.execute(toRunnable.apply(evt2, 9, 9000));

        ObservableList<ReactivePoint> points = FXCollections.observableArrayList();
        points.add(new ReactivePoint(evt1, evt2));
        return points;
    }

    private static final class ReactivePoint {
        private final EventStream<Integer> x;
        private final EventStream<Integer> y;

        ReactivePoint(EventStream<Integer> x, EventStream<Integer> y) { 
            this.x = x;
            this.y = y;

            x.subscribe(v -> System.out.println("Emitting x: " + v));
            y.subscribe(v -> System.out.println("Emitting y: " + v));
        }
        public EventStream<Integer> getX() {
            return x;
        }
        public EventStream<Integer> getY() { 
            return y;
        }
    }

    private static final class ReactiveTable extends TableView<ReactivePoint> {

        @SuppressWarnings("unchecked")
        private ReactiveTable(ObservableList<ReactivePoint> reactivePoints) { 
            this.setItems(reactivePoints);


            System.out.println("Constructor is happening on FX THREAD: " + Platform.isFxApplicationThread());

            TableColumn<ReactivePoint,Number> xCol = new TableColumn<>("X");
            xCol.setCellValueFactory(cb -> { 
                System.out.println("CellValueFactory for X called on FX THREAD: " + Platform.isFxApplicationThread());
                return cb.getValue().getX().map(x -> (Number) x).toBinding(-1);
                }
            );

            TableColumn<ReactivePoint,Number> yCol = new TableColumn<>("Y");
            yCol.setCellValueFactory(cb -> {
                System.out.println("CellValueFactory for Y called on FX THREAD: " + Platform.isFxApplicationThread());
                return cb.getValue().getY().map(y -> (Number) y).toBinding(-1);
            }
        );

            this.getColumns().addAll(xCol, yCol);
        }
    }

    public static void main(String[] args) { 
        launch(args);
    }
}

@TomasMikula
Copy link
Owner

There are several problems with this code, but the one that is preventing the updates goes like this:

  • TableView asks for the cell value ObservableValue, so you create a binding with initial value -1. TableView displays -1 and listens for invalidations of this binding.
  • The binding is invalidated (value changing to 5). The TableView notices the invalidation and does the full layout. In this process, it first stops observing the cell value binding (which now already holds 5) and then uses the cellValueFactory to get the binding again, so you create a new binding with initial value -1 (which does not get updated until the next event is emitted). So -1 is now the "new" value displayed by the TableView.

Other problems are:

  • The Binding returned from toBinding should be disposed using its dispose() method when no longer used. Otherwise, it will stay in memory and keep being updated as long as, in your case, the underlying EventSource is not garbage collected. In your example, you create a new Binding on each layout and they are never garbage collected, because the underlying EventSource is still in use.
  • You should use Platform.runLater to push events into EventSource, since the event handlers then operate on the UI.

A solution is creating one Binding per ReactivePoint:

    private static final class ReactivePoint {
        private final Binding<Number> x;
        private final Binding<Number> y;

        ReactivePoint(EventStream<Integer> x, EventStream<Integer> y) {
            this.x = x.<Number>map(i -> i).toBinding(-1);
            this.y = y.<Number>map(i -> i).toBinding(-1);
        }
        public Binding<Number> getX() {
            return x;
        }
        public Binding<Number> getY() {
            return y;
        }
        public void dispose() {
            x.dispose();
            y.dispose();
        }
    }

Note also that I added the dispose() method which should be called when the ReactivePoint is no longer used.

@thomasnield
Copy link
Author

Okay wow, this worked great. That makes complete sense. So I guess what has to happen then is the Binding must exist and persist outside the cell factory, otherwise invalidation will cause a new Binding to be created like you said. I'll have to think about my design patterns to accommodate this.

@thomasnield
Copy link
Author

I was also able to use an RxJava Observable and convert it to an EventStream, and everything worked as long as I kept the Binding construction outside the cellValueFactory. Thanks for your help Tomas. This was driving me up the wall and it you're one of the few people who can answer these questions.

@TomasMikula
Copy link
Owner

I think it helps to think of "cellValueFactory" more like "column value extractor"—a function that knows where in row data to find the (observable) value for a certain column. So it makes sense to persist it with the row data.

It is not necessarily wrong for this function to create the observable column value on the fly on each call, except in your case the binding it creates will initially be -1 and when it changes the TableView will ask for the observable cell value again, but when you create a new one it will again be -1, since the event that changed the previously created binding to 5 is already gone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants