Bug 478611 - [DataBinding] Create snippets for the SideEffect class
Summary: [DataBinding] Create snippets for the SideEffect class
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.6   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: 4.15 M1   Edit
Assignee: Simon Scholz CLA
QA Contact:
URL:
Whiteboard:
Keywords: example
Depends on: 478456
Blocks:
  Show dependency tree
 
Reported: 2015-09-29 05:12 EDT by Simon Scholz CLA
Modified: 2020-01-06 04:03 EST (History)
3 users (show)

See Also:
sxenos: review+

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Scholz CLA 2015-09-29 05:12:16 EDT 
In Bug 478456 a new SideEffect class is being created and it would be useful to provide some snippets with examples for the usage of the SideEffect class.
Comment 1 Eclipse Genie CLA 2015-09-29 05:12:46 EDT 
New Gerrit change created: https://git.eclipse.org/r/56927
Comment 2 Eclipse Genie CLA 2015-11-03 21:13:28 EST 
New Gerrit change created: https://git.eclipse.org/r/59623
Comment 3 Stefan Xenos CLA 2015-11-11 20:58:52 EST 
Since we still have the hashcode and equals problems with Observables, the safest use of SideEffect will probably be for one-way bindings (initializing labels, the contents of a listbox, etc.) I'd suggest adding such an example.
Comment 4 Stefan Xenos CLA 2015-11-11 20:59:38 EST 
FYI, the SideEffect class was committed to master just now, so we can start committing these as well.
Comment 5 Lars Vogel CLA 2015-12-02 06:05:51 EST 
Simon, any plans to finish this for M4?
Comment 7 Eclipse Genie CLA 2016-03-07 21:29:25 EST 
New Gerrit change created: https://git.eclipse.org/r/67929
Comment 8 Eclipse Genie CLA 2016-03-07 22:40:41 EST 
New Gerrit change created: https://git.eclipse.org/r/67931
Comment 10 Eclipse Genie CLA 2016-03-09 14:34:23 EST 
New Gerrit change created: https://git.eclipse.org/r/68082
Comment 11 Eclipse Genie CLA 2016-03-09 23:34:46 EST 
New Gerrit change created: https://git.eclipse.org/r/68123
Comment 14 Stefan Xenos CLA 2016-03-10 00:49:20 EST 
I notice in one of the recent samples that the ISideEffect version of the validation code is more lines of code than the DataBindingContext version.

Much of the extra code is due to the need to explicitly dispose the ISideEffects.

I'd suggest we create the following helper method:

ControlSideEffect {
ISideEffect create(Control c, Supplier<T>, Consumer<T>)
}

It would create a new side effect which gets disposed along with the given control. For example:

ControlSideEffect.create(okButton, () -> validationStatus.getValue().isOK(),
okButton::setEnabled);

That would eliminate the need for explicit disposal code, and (in some cases) the need for a local variable to hold the side effect.

The new utility could live in a new utility class, SWTObservables, or WidgetProperties. I'm not sure where the best place for it is.
Comment 15 Stefan Xenos CLA 2016-03-10 00:53:20 EST 
We could also create a new utility for attaching side effects to workbench parts. It could pause and resume the side-effect when the part is hidden, and dispose the side-effect when the part is closed.

I'm not sure what the prototype should be, though, since I'm unsure how E4 parts track their own visibility.
Comment 16 Simon Scholz CLA 2016-03-10 08:55:04 EST 
(In reply to Stefan Xenos from comment #14)
> I notice in one of the recent samples that the ISideEffect version of the
> validation code is more lines of code than the DataBindingContext version.
> 
> Much of the extra code is due to the need to explicitly dispose the
> ISideEffects.

Basically the DataBindingContext is a container for all local bindings. When the DataBindingContext is disposed all Bindings will be disposed too.
What do you think of having such a container for ISideEffects as well?

public class SideEffectContext implements ISideEffect {

	private IObservableList<ISideEffect> sideEffects;

	public SideEffectContext() {
		this(Realm.getDefault());
	}

	public SideEffectContext(Realm realm) {
		sideEffects = new WritableList<>(realm);
	}

	@Override
	public void dispose() {
		sideEffects.forEach(s -> s.dispose());
		sideEffects.clear();
	}

	@Override
	public void pause() {
		sideEffects.forEach(s -> s.pause());
	}

	@Override
	public void resume() {
		sideEffects.forEach(s -> s.resume());
	}

	@Override
	public void resumeAndRunIfDirty() {
		sideEffects.forEach(s -> s.resumeAndRunIfDirty());
	}

	@Override
	public void runIfDirty() {
		sideEffects.forEach(s -> s.runIfDirty());
	}

	public ISideEffect createPaused(Runnable runnable) {
		ISideEffect sideEffect = ISideEffect.createPaused(runnable);
		sideEffects.add(sideEffect);

		return sideEffect;
	}

	public ISideEffect createPaused(Realm realm, Runnable runnable) {
		ISideEffect sideEffect = ISideEffect.createPaused(realm, runnable);
		sideEffects.add(sideEffect);

		return sideEffect;
	}

	public ISideEffect create(Runnable runnable) {
		ISideEffect sideEffect = ISideEffect.create(runnable);
		sideEffects.add(sideEffect);

		return sideEffect;
	}

	public <T> ISideEffect create(Supplier<T> supplier, Consumer<T> consumer) {
		ISideEffect sideEffect = ISideEffect.create(supplier, consumer);
		sideEffects.add(sideEffect);

		return sideEffect;
	}

	public <T> ISideEffect createResumed(Supplier<T> supplier, Consumer<T> consumer) {
		ISideEffect sideEffect = ISideEffect.createResumed(supplier, consumer);
		sideEffects.add(sideEffect);

		return sideEffect;
	}

	public <T> ISideEffect consumeOnceAsync(Supplier<T> supplier, Consumer<T> consumer) {
		ISideEffect sideEffect = ISideEffect.consumeOnceAsync(supplier, consumer);
		sideEffects.add(sideEffect);

		return sideEffect;
	}
}

With this approach you can use it like this:

SideEffectContext sideEffectContext = new SideEffectContext();

sideEffectContext.create(() -> validationStatus.getValue().isOK(),
okButton::setEnabled);

sideEffectContext.create(person::getFirstName, personFirstNameText::setText);
sideEffectContext.create(personFirstNameText::getText, person::setFirstName);

// disposed all sideeffects
personFirstNameText.addDisposeListener(e -> sideEffectContext.dispose());

Then we could also have a ControlSideEffectContext with a third constructor in JFace Databinding, like this:


public class ControlSideEffectContext extends SideEffectContext {
	public ControlSideEffectContext() {
		this(Realm.getDefault());
	}

	public ControlSideEffectContext(Realm realm) {
		this(realm, null);
	}

	public ControlSideEffectContext(Realm realm, Control disposeableControl) {
		super(realm);
                disposeableControl.addDisposeListener(e -> ControlSideEffectContext.this.dispose());
	}
}

... so that even the disposal is automatically managed by using ControlSideEffectContext.

Concerning E4 Parts, we could then put this SideEffectContext into the context of a part and create a model addon, which is able to pause all SideEffects in case the part is hidden in the stack.

@Inject
@Optional
public void subscribeTopicSelectedElement(@EventTopic
    (UIEvents.ElementContainer.TOPIC_SELECTEDELEMENT) Event event) {
  Object newValue = event.getProperty(EventTags.NEW_VALUE);
  Object oldValue = event.getProperty(EventTags.OLD_VALUE);


  if (oldValue instanceof MPart) {
      MPart oldPart = (MPart) oldValue;

     ISideEffect oldPartsSideEffect = oldPart.getContext().get(ISideEffect.class);
     if(oldPartsSideEffect != null) {
       oldPartsSideEffect.pause();
     } 
  }

  if (newValue instanceof MPart) {
      MPart newPart = (MPart) newValue;

      ISideEffect newPartsSideEffect = newPart.getContext().get(ISideEffect.class);
      if(newPartsSideEffect != null) {
        newPartsSideEffect.resume();
      } 
  }

}
Comment 17 Eclipse Genie CLA 2016-03-12 18:44:34 EST 
New Gerrit change created: https://git.eclipse.org/r/68283
Comment 18 Eclipse Genie CLA 2016-05-11 18:23:55 EDT 
New Gerrit change created: https://git.eclipse.org/r/72578
Comment 21 Dani Megert CLA 2016-05-12 06:33:54 EDT 
Please revert those changes. No changes are allowed at this point. Please read https://dev.eclipse.org/mhonarc/lists/platform-ui-dev/msg07082.html

"
As previously mentioned by David, tonight 20:00 EDT will be our RC1 candidate build. This means, no further changes must be committed or merged after that.
"
Comment 22 Eclipse Genie CLA 2016-05-12 07:06:48 EDT 
New Gerrit change created: https://git.eclipse.org/r/72613
Comment 24 Dani Megert CLA 2016-05-12 07:47:21 EDT 
(In reply to Eclipse Genie from comment #23)
> Gerrit change https://git.eclipse.org/r/72613 was merged to [master].
> Commit:
> http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=8c9039bbe6cd389f7a517182efabb43b545aae88
> 

Thanks. Please also revert the other change.
Comment 25 Dani Megert CLA 2016-05-12 10:59:14 EDT 
(In reply to Dani Megert from comment #24)
> (In reply to Eclipse Genie from comment #23)
> > Gerrit change https://git.eclipse.org/r/72613 was merged to [master].
> > Commit:
> > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=8c9039bbe6cd389f7a517182efabb43b545aae88
> > 
> 
> Thanks. Please also revert the other change.

Not good. The rebuild started and now this bug is partially in RC1 and RC2.
Comment 26 Eclipse Genie CLA 2016-05-12 11:05:56 EDT 
New Gerrit change created: https://git.eclipse.org/r/72637
Comment 28 Simon Scholz CLA 2016-05-12 11:09:29 EDT 
Sorry I was busy almost all afternoon. I reverted it right now. 
I thought for example projects it would still be ok to merge.
Comment 29 Dani Megert CLA 2016-05-12 11:21:37 EDT 
(In reply to Simon Scholz from comment #28)
> Sorry I was busy almost all afternoon. 

Next time, please just tell us that here, and someone else takes care of it. That's not a problem.


> I reverted it right now. 

Unfortunately, this makes it even worse. You could have checked
http://git.eclipse.org/c/platform/eclipse.platform.ui.git/
to see that your change was already picked up by the 10:00 rebuild, i.e. this is now in RC1 (unless we have to rebuild again), but you've reverted it after that build.


> I thought for example projects it would still be ok to merge.

Nope, no commit or merge is allowed, except for those that get announced as critical issues on the platform-releng mailing list. We're forgiving commits that do not affect the build, e.g. changes to N&N, webpage, etc.
Comment 30 Eclipse Genie CLA 2016-05-16 18:45:42 EDT 
New Gerrit change created: https://git.eclipse.org/r/72852
Comment 32 Eclipse Genie CLA 2016-05-16 19:07:22 EDT 
New Gerrit change created: https://git.eclipse.org/r/72854
Comment 35 Simon Scholz CLA 2016-05-17 03:35:32 EDT 
Done with creating snippets for now.
Comment 36 Lars Vogel CLA 2019-11-27 05:48:29 EST 
Still one open Gerrit.
Comment 38 Lars Vogel CLA 2020-01-06 04:03:39 EST 
Thanks Simon and Jens.