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

[Feature Request] Vetoable Listeners, LiveSet & LiveMap #23

Open
DirkToewe opened this issue Jul 1, 2015 · 7 comments
Open

[Feature Request] Vetoable Listeners, LiveSet & LiveMap #23

DirkToewe opened this issue Jul 1, 2015 · 7 comments

Comments

@DirkToewe
Copy link

Hello,

I have a scene graph-like data structure that heavily relies on observable properties to keep the components mutually updated. I would like to use listeners to prevent/veto invalid changes of properties. Using a Change(Performed)Listener to change back values is not satisfactory for two reasons:

  • Read-only properties could not veto.
  • The intermediate invalid value fired as Event could cause consistency issues elsewhere

Is there any way to improve Val and Var in the ReactFX library in a way that also allows for vetoable listeners? I understand that this would not work in interaction with FX properties as they do not support VetoListeners. But in my application the data-stucture is separate from the JavaFX Views so I would like to use the ReactFX properties as "standalone".

I have written my own little property library that has an additional listener that is listening to events before changes are performed (ChangeProposedListener). The problem is my library is non-lazy, will create memory leaks left and right and is over all not nearly as awesome and powerful as the ReactFX library (It was created in 2 hours....).

In addition to that I would also require both a vetoable LiveSet and LiveMap as they are used in the model as well.

I feel like this is too much to ask for. A the same time I believe many other people could use vetoability as well. Just look at the horrible abomination of a VetoableListDecorator that can be found in the JavaFX library.

cheers
Dirk

@TomasMikula
Copy link
Owner

Hi Dirk,

can you perhaps give a small example of what you mean by vetoable listeners and how you would use them (e.g. to prevent inconsistency)?

The original reason why I didn't include LiveSet or LiveMap is that I wasn't sure what should the semantics of map (mapKeys for LiveMap) operation be. It is tricky when the map is not injective, i.e. when it maps two or more elements to one. A solution could be that the result of map/mapKeys is a MultiSet/MultiMap. map also cannot be implemented as a view of the original Set/Map.

More recently, I'm not convinced the approach taken by JavaFX (and adopted by ReactFX's LiveList) of having a special kind of observable objects (ObservableList, ObservableSet, ...) for each collection type is the right approach. Maybe we should instead search for a generalization of ObservableValue/Val that can be specialized to conveniently handle collections.

@DirkToewe
Copy link
Author

Two possibilities:

Either throwing Exceptions:

Var<Float> var = simpleVar(1f);
Val<Float> val = var.map( x -> x*x );
val.addChangeProposedListener(
  (oldVal,newVal) -> { if(newVal > 3) throw new IllegalArgumentException(); }
);
var.setValue(2); // <- should throw the IllegalArgumentException

A maybe cleaner way would be to return a Vote:

Var<Float> var = simpleVar(1f);
Val<Float> val = var.map( x -> x*x );
val.addChangeProposedListener(
  (oldVal,newVal) -> newVal > 3 ? new Veto("square too large") : new Approval().onRejection( () -> {} ).onPass( () -> {} }
);
var.setValue(2); // <- should throw the IllegalArgumentException
var.proposeValue(2); // <- should return some kind of answer Object

Hope this makes sense.

Also I would be perfectly happy if LiveSet.map would throw an UnsupportedOperationException or would not exist at all.

I would argue that JavaFX's separation between ObservableList and ObservableValue does not go far enough: ObservableValue[ObservableList[E]] and ObservableList[E] have completely different purposes in my opinion and should not be mixed up. ObservableValue<ObservableList[E]> is supposed to keep track of the reference to a list. While ObservableList[E] is supposed to keep track of the entries of a list.

@TomasMikula
Copy link
Owner

In both cases, the result of var.setValue/var.proposeValue depends on what listeners are attached to val. Could you handle the exception/negative answer in any meaningful way?

My view is this: If var should never be set to anything bigger than sqrt(3), it is OK to throw an exception, because it is a bug that should be fixed. If there is no restriction on the value held by var, but val should never be greater than 3, then filter out values greater than 3 from val (leaving var set to whatever).

@DirkToewe
Copy link
Author

In 95% of all cases that is absolutely true, but:

  • Let's say it's a Bug. It may still be important that the Model stays intact. What if you need to do a backup of the data for recovery? You cannot save a corrupted data model. Let's say the data is safety relevant in some way. A machine is now running with out-of-bounds parameters. Not for long since an emergency shutdown is issued. but for a second it will.
  • Let's say it's not a Bug. In my case it's a user input. It is not controllable at compile-time so you have to handle it at runtime in a meaningful way. If the data stays intact I at most have to worry about reverting some TextField. I do not have to try and fix some complex dependency in my model.

In my opinion it is bad practice to throw Exceptions after changes have been performed.

I see the val as a user/consumer of the var. It has the right to complain but no right to change it.

@TomasMikula
Copy link
Owner

Let's say it's a Bug. It may still be important that the Model stays intact. What if you need to do a backup of the data for recovery? You cannot save a corrupted data model.

It seems to me that whatever check you do to veto an update, you could do before pushing an invalid value into the dataflow.
Moreover, by the time you detect an inconsistency, the data may have been corrupted already, by a bug that went undetected. Therefore, I don't see any rollback buying you much.

In my case it's a user input.

You should validate all user input in the first place.

Anyway, so you imagine a dry run for each update and if everything goes well (no veto), then do the actual update? What if the actual update causes an exception?

@DirkToewe
Copy link
Author

You should validate all user input in the first place.

The problem is that the validity of a text value depends on where it is in the whole data structure. Imagine a name of a tree node that needs to be unique within the parent. I would like to not have to traverse the datastructure inside the JavaFX components. Checks become a lot more complex as well as they have to aquire context/traverse the data structure.

Anyway, so you imagine a dry run for each update and if everything goes well (no veto), then do the actual update? What if the actual update causes an exception?

Sh** hits the fan 😄 ... There is no way to make this bullet proof. But my idea is that simple small checks by individual components are simpler and safer than complex queries before pushing a change to the data structure. It is also more modular and extensible. The data model guy can do things the GUI guy does not even have to think about. The different data guys can write modular checks.

@TomasMikula
Copy link
Owner

I see your motivation. Still, the idea that values accepted by a property depend on who is listening seems fragile to me

Modular checks? Sure! You can construct a Val<Predicate<String>>, where the current predicate would be constructed based on the currently selected tree node. Then use that predicate to test the input. The predicate would be updated appropriately as you traverse the tree.

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