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

Change DelegateCommands parameter "executableBinding" from ObservableBooleanValue to ObservableValue<Boolean> #384

Closed
simonsilvalauinger opened this issue May 3, 2016 · 2 comments
Labels
Milestone

Comments

@simonsilvalauinger
Copy link

It's more abstract and give you the freedom to do inline bindings.
This code

public class MyViewModel implements ViewModel {
   private final ObjectProperty<Object> op = new SimpleObjectProperty<Object>();
   private final BooleanProperty bp = new SimpleBooleanProperty(false);
   private final Command c = new DelegateCommand(() -> new Action() {
      @Override
      protected void action() throws Exception {
         // do whatever you want
      }
   }, bp);

   public MyViewModel() {
      bp.bind(EasyBind.monadic(op).map(Objects::nonNull));
   }
}

would be reduced to

public class MyViewModel implements ViewModel {
   private final ObjectProperty<Object> op = new SimpleObjectProperty<Object>();
   private final Command c = new DelegateCommand(() -> new Action() {
      @Override
      protected void action() throws Exception {
         // do whatever you want
      }
   }, EasyBind.monadic(op).map(Objects::nonNull));
}
@sialcasa
Copy link
Owner

sialcasa commented May 3, 2016

I'm not sure but this binding may gets garbage collected with the current implementation. Can you please create a PR for it with tests that proves, that this works?

manuel-mauky added a commit that referenced this issue May 23, 2016
@manuel-mauky
Copy link
Collaborator

manuel-mauky commented May 23, 2016

Hi @simonsilvalauinger
thanks for your feature request. I think that @sialcasa is correct and your code example wouldn't work in practice because the Binding EasyBind.monadic(op).map(Objects::nonNull) will be garbage collected because no reference exists.

But besides this of cause your initial request is valid and the parameter could also be of type ObservableValue<Boolean>. The reason for using ObservableBooleanValue in the first place was that the internal implementation is easier because it provides a binding for logical AND.

In #389 I have created an low-level-binding instead and changed the parameter type.

Existing code will not break because ObservableBooleanValue extends ObservableValue<Boolean>.

@manuel-mauky manuel-mauky added this to the 1.5.0 milestone May 23, 2016
manuel-mauky added a commit that referenced this issue May 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants