RE: Move to JIRA [was: Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method]

2014-01-26 Thread Seeon Birger
Hi Anthony,

Thanks for sending a request for applying the multi-threaded plug-in.

As for the email plug-in mentioned below, using it does not mean sending all 
JIRA's automatically to a single mailing list.. 
On the contrary it can be used as a selective tool for sharing selective JIRAs 
to selective people/groups. This might be handy in case an issue was previously 
discussed in JIRA and at a point you want to share with specific recipients.
Anyway if the current watch list mechanism functionality suffices that's it's 
fine with me.. :)

Regards,
Seeon

-Original Message-
From: Anthony Petrov 
Sent: Friday, January 24, 2014 1:07 PM
To: Seeon Birger; Steve Northover
Cc: John Hendrikx; openjfx-dev@openjdk.java.net
Subject: Re: Move to JIRA [was: Re: [8u] API Request: RT-25613, ObservableValue 
should have a hasListener(listener) method]

Hi Seeon,

I don't think that posting updates from all our JIRAs to a single mailing list 
is a good idea because the messages traffic would be huge. 
Each JIRA provides a watch list, so people can follow specific bugs that 
they're interested in. I believe that this functionality is sufficient for all 
reasonable purposes.

However, your suggestion about using threaded comments sounds interesting. 
We've sent a request to our Infrastructure team to evaluate the plugin and 
consider adding it to our JIRA instance. Note that this is not going to happen 
overnight and could take some time, but the request is filed at least. Thanks 
for the suggestion!

--
best regards,
Anthony

On 1/23/2014 9:09 PM, Seeon Birger wrote:
 Steve,

 I wonder if we could take advantage of available plug-ins for JIRA.

 I particular I found this one which enables threaded comments for JIRA:
 https://marketplace.atlassian.com/plugins/com.atlassian.jira.threadedc
 omments.threaded-comments

 Also interesting is the following which make it easy to put JIRA updates on 
 mailing lists in a flexible and customizable way:
 https://marketplace.atlassian.com/plugins/com.metainf.jira.plugin.emai
 lissue

 What do you think?

 Seeon



 -Original Message-
 From: Stephen F Northover
 Sent: Thursday, January 23, 2014 12:45 AM
 To: John Hendrikx; openjfx-dev@openjdk.java.net
 Subject: Re: Move to JIRA [was: Re: [8u] API Request: RT-25613, 
 ObservableValue should have a hasListener(listener) method]

 Hi John,

 The goal is not to end the discussion!

 It's a trade off.  Mailing lists are good because they provide a threaded 
 discussion.  JIRA is bad because it is not threaded.  JIRA has the advantage 
 that it captures data in a single place and provides a good history of why a 
 decision was made.

 There's no right answer here but the policy that the FX committers is using 
 is to try to capture as much as possible in JIRA.

 Steve

 On 2014-01-22 5:29 PM, John Hendrikx wrote:
 Unfortunately, discussing things in JIRA works very poorly and is a 
 good way to end a productive discussion IMHO.  Mailinglists are much 
 better suited to the task, as thousands of interesting mailinglists 
 accross many developer communities will atest to.

 Keeping a record is good, aren't these mailinglists archived?

 --John

 On 22/01/2014 18:47, Daniel Blaukopf wrote:
 Hi Martin, Randahl, Tom, Richard, Tomas and Ali,

 This is a productive discussion, but once we get to this level of 
 detail JIRA is the place to have it, so that we don't lose our 
 record of it. Would you continue the discussion on
 https://javafx-jira.kenai.com/browse/RT-25613 ?

 See
 https://wiki.openjdk.java.net/display/OpenJFX/Code+Reviews#CodeRevie
 w s-TechnicalDiscussionsandCodeReviews

 Thanks,
 Daniel

 On Jan 22, 2014, at 7:23 PM, Stephen F 
 Northoversteve.x.northo...@oracle.com  wrote:

 If we add this API, I like addListener(InvalidationListener,
 boolean) better than ensureListener().

 Steve

 On 2014-01-22 8:20 AM, Ali Ebrahimi wrote:
 I suggest adding another overload for addListener method taking 
 boolean parameter  duplicateAllowed or duplicateNotAllowed.


 On Wed, Jan 22, 2014 at 3:00 PM, Richard
 Bairrichard.b...@oracle.comwrote:

 The default implementation (for Observable) would look like this:

 public default void ensureListener(InvalidationListener listener) {
  removeListener(listener);
  addListener(listener);
 }

 subclasses might do something more effective. The same would 
 apply to ObservableValue and ChangeListener and 
 Observable[List|Set|Map] and [List|Set|Map]ChangeListener.
 Well this would destroy the order! I expect listeners to be 
 called in the correct order not?
 That's a good point :-(

 Why doing a remove and not simply check if the
 listener has already been added?
 Because there is no way to check, except in the implementation.
  From the
 Observable interface level, there is no way to a) force all
 implementations
 of the interface to implement the method correctly (without
 breaking source
 compatibility anyway), or b) to provide a reasonable default
 implementation.

 Maybe

RE: Move to JIRA [was: Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method]

2014-01-23 Thread Seeon Birger
Steve,

I wonder if we could take advantage of available plug-ins for JIRA.

I particular I found this one which enables threaded comments for JIRA:
https://marketplace.atlassian.com/plugins/com.atlassian.jira.threadedcomments.threaded-comments

Also interesting is the following which make it easy to put JIRA updates on 
mailing lists in a flexible and customizable way:
https://marketplace.atlassian.com/plugins/com.metainf.jira.plugin.emailissue

What do you think?

Seeon



-Original Message-
From: Stephen F Northover 
Sent: Thursday, January 23, 2014 12:45 AM
To: John Hendrikx; openjfx-dev@openjdk.java.net
Subject: Re: Move to JIRA [was: Re: [8u] API Request: RT-25613, ObservableValue 
should have a hasListener(listener) method]

Hi John,

The goal is not to end the discussion!

It's a trade off.  Mailing lists are good because they provide a threaded 
discussion.  JIRA is bad because it is not threaded.  JIRA has the advantage 
that it captures data in a single place and provides a good history of why a 
decision was made.

There's no right answer here but the policy that the FX committers is using is 
to try to capture as much as possible in JIRA.

Steve

On 2014-01-22 5:29 PM, John Hendrikx wrote:
 Unfortunately, discussing things in JIRA works very poorly and is a 
 good way to end a productive discussion IMHO.  Mailinglists are much 
 better suited to the task, as thousands of interesting mailinglists 
 accross many developer communities will atest to.

 Keeping a record is good, aren't these mailinglists archived?

 --John

 On 22/01/2014 18:47, Daniel Blaukopf wrote:
 Hi Martin, Randahl, Tom, Richard, Tomas and Ali,

 This is a productive discussion, but once we get to this level of 
 detail JIRA is the place to have it, so that we don't lose our record 
 of it. Would you continue the discussion on
 https://javafx-jira.kenai.com/browse/RT-25613 ?

 See
 https://wiki.openjdk.java.net/display/OpenJFX/Code+Reviews#CodeReview
 s-TechnicalDiscussionsandCodeReviews

 Thanks,
 Daniel

 On Jan 22, 2014, at 7:23 PM, Stephen F 
 Northoversteve.x.northo...@oracle.com  wrote:

 If we add this API, I like addListener(InvalidationListener,
 boolean) better than ensureListener().

 Steve

 On 2014-01-22 8:20 AM, Ali Ebrahimi wrote:
 I suggest adding another overload for addListener method taking 
 boolean parameter  duplicateAllowed or duplicateNotAllowed.


 On Wed, Jan 22, 2014 at 3:00 PM, Richard 
 Bairrichard.b...@oracle.comwrote:

 The default implementation (for Observable) would look like this:

 public default void ensureListener(InvalidationListener listener) {
 removeListener(listener);
 addListener(listener);
 }

 subclasses might do something more effective. The same would 
 apply to
 ObservableValue and ChangeListener and Observable[List|Set|Map] and
 [List|Set|Map]ChangeListener.
 Well this would destroy the order! I expect listeners to be 
 called in
 the correct order not?
 That's a good point :-(

 Why doing a remove and not simply check if the
 listener has already been added?
 Because there is no way to check, except in the implementation. 
 From the
 Observable interface level, there is no way to a) force all 
 implementations
 of the interface to implement the method correctly (without 
 breaking source
 compatibility anyway), or b) to provide a reasonable default 
 implementation.

 Maybe this is one of those things we can't fix on the Observable 
 interface
 and just have to provide implementations of on our concrete 
 properties.

 Richard




[8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method

2014-01-22 Thread Martin Sladecek

Hi all,
I would like to start discussion about an addition to API in Observable, 
ObservableValue and all Observable collections.
There were multiple requests for a way how to avoid duplicates in 
listeners lists. The way RT-25613 solves this is that it introduces 
public boolean hasListener(ListenerType listener) which would return 
true if the provided listener is already registered.


This has one significant drawback that all of Observable* are actually 
interfaces. Means we can only add hasListener as a defender method. The 
problem is with the default implementation. We cannot return anything 
meaningful, so we have to throw an UnsupportedOperationException. The 
problem is that this might blow up unexpectedly when some older 
Observable implementation is used. Also, it might be easy to miss when 
implementing the interface, since the IDE might not force you to 
implement it.


So as an alternative solution, I propose adding something like:

ensureListener(ListenerType listener)

which would make sure the listener is on the list and if a listener is 
already present, the number of times listener is registered on the 
Observable will NOT grow after this call.


The default implementation (for Observable) would look like this:

public default void ensureListener(InvalidationListener listener) {
removeListener(listener);
addListener(listener);
}

subclasses might do something more effective. The same would apply to 
ObservableValue and ChangeListener and Observable[List|Set|Map] and 
[List|Set|Map]ChangeListener.


What do you think?

JIRA link: https://javafx-jira.kenai.com/browse/RT-25613

-Martin


Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method

2014-01-22 Thread Randahl Fink Isaksen
Hi Martin

While I agree your proposed solution would work, I still don’t understand why 
JavaFX should keep on supporting duplicates in listener collections. Can anyone 
come up with just 1 example of an application that might be depending on having 
two listeners on the same Observable? E.g. this kind of code:

myObservable.addListener(myChangeListener); //add it
myObservable.addListener(myChangeListener); //add it again

In what kind of situation would this sort of code make any sense?

If we all feel confident that the presence of duplicates listeners is always an 
error, I warmly recommend changing the API to be duplicate free.

Yours

Randahl




On 22 Jan 2014, at 11:07, Martin Sladecek martin.slade...@oracle.com wrote:

 Hi all,
 I would like to start discussion about an addition to API in Observable, 
 ObservableValue and all Observable collections.
 There were multiple requests for a way how to avoid duplicates in listeners 
 lists. The way RT-25613 solves this is that it introduces public boolean 
 hasListener(ListenerType listener) which would return true if the provided 
 listener is already registered.
 
 This has one significant drawback that all of Observable* are actually 
 interfaces. Means we can only add hasListener as a defender method. The 
 problem is with the default implementation. We cannot return anything 
 meaningful, so we have to throw an UnsupportedOperationException. The problem 
 is that this might blow up unexpectedly when some older Observable 
 implementation is used. Also, it might be easy to miss when implementing the 
 interface, since the IDE might not force you to implement it.
 
 So as an alternative solution, I propose adding something like:
 
 ensureListener(ListenerType listener)
 
 which would make sure the listener is on the list and if a listener is 
 already present, the number of times listener is registered on the Observable 
 will NOT grow after this call.
 
 The default implementation (for Observable) would look like this:
 
 public default void ensureListener(InvalidationListener listener) {
removeListener(listener);
addListener(listener);
 }
 
 subclasses might do something more effective. The same would apply to 
 ObservableValue and ChangeListener and Observable[List|Set|Map] and 
 [List|Set|Map]ChangeListener.
 
 What do you think?
 
 JIRA link: https://javafx-jira.kenai.com/browse/RT-25613
 
 -Martin



Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method

2014-01-22 Thread Martin Sladecek
The reason why this was decided this way is simple : performance. You 
usually don't (try to) add a listener twice, so in most cases it doesn't 
make sense to check for duplicates every time a listener is added. So we 
currently leave the burden of avoiding duplicates on the developer.


-Martin

On 01/22/2014 11:23 AM, Randahl Fink Isaksen wrote:

Hi Martin

While I agree your proposed solution would work, I still don’t understand why 
JavaFX should keep on supporting duplicates in listener collections. Can anyone 
come up with just 1 example of an application that might be depending on having 
two listeners on the same Observable? E.g. this kind of code:

myObservable.addListener(myChangeListener); //add it
myObservable.addListener(myChangeListener); //add it again

In what kind of situation would this sort of code make any sense?

If we all feel confident that the presence of duplicates listeners is always an 
error, I warmly recommend changing the API to be duplicate free.

Yours

Randahl




On 22 Jan 2014, at 11:07, Martin Sladecek martin.slade...@oracle.com wrote:


Hi all,
I would like to start discussion about an addition to API in Observable, 
ObservableValue and all Observable collections.
There were multiple requests for a way how to avoid duplicates in listeners 
lists. The way RT-25613 solves this is that it introduces public boolean 
hasListener(ListenerType listener) which would return true if the provided 
listener is already registered.

This has one significant drawback that all of Observable* are actually interfaces. Means 
we can only add hasListener as a defender method. The problem is with the default 
implementation. We cannot return anything meaningful, so we have to throw an 
UnsupportedOperationException. The problem is that this might blow up unexpectedly when 
some older Observable implementation is used. Also, it might be easy to miss 
when implementing the interface, since the IDE might not force you to implement it.

So as an alternative solution, I propose adding something like:

ensureListener(ListenerType listener)

which would make sure the listener is on the list and if a listener is already 
present, the number of times listener is registered on the Observable will NOT 
grow after this call.

The default implementation (for Observable) would look like this:

public default void ensureListener(InvalidationListener listener) {
removeListener(listener);
addListener(listener);
}

subclasses might do something more effective. The same would apply to 
ObservableValue and ChangeListener and Observable[List|Set|Map] and 
[List|Set|Map]ChangeListener.

What do you think?

JIRA link: https://javafx-jira.kenai.com/browse/RT-25613

-Martin




Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method

2014-01-22 Thread Tom Schindl
On 22.01.14 11:07, Martin Sladecek wrote:
 Hi all,
 I would like to start discussion about an addition to API in Observable,
 ObservableValue and all Observable collections.
 There were multiple requests for a way how to avoid duplicates in
 listeners lists. The way RT-25613 solves this is that it introduces
 public boolean hasListener(ListenerType listener) which would return
 true if the provided listener is already registered.
 
 This has one significant drawback that all of Observable* are actually
 interfaces. Means we can only add hasListener as a defender method. The
 problem is with the default implementation. We cannot return anything
 meaningful, so we have to throw an UnsupportedOperationException. The
 problem is that this might blow up unexpectedly when some older
 Observable implementation is used. Also, it might be easy to miss when
 implementing the interface, since the IDE might not force you to
 implement it.
 
 So as an alternative solution, I propose adding something like:
 
 ensureListener(ListenerType listener)
 
 which would make sure the listener is on the list and if a listener is
 already present, the number of times listener is registered on the
 Observable will NOT grow after this call.
 
 The default implementation (for Observable) would look like this:
 
 public default void ensureListener(InvalidationListener listener) {
 removeListener(listener);
 addListener(listener);
 }
 
 subclasses might do something more effective. The same would apply to
 ObservableValue and ChangeListener and Observable[List|Set|Map] and
 [List|Set|Map]ChangeListener.

Well this would destroy the order! I expect listeners to be called in
the correct order not? Why doing a remove and not simply check if the
listener has already been added?

Tom


Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method

2014-01-22 Thread Randahl Fink Isaksen
Hi Martin

Then I respectfully disagree with this design decision. In my point of view, 
choosing performance over ease of use is rarely a good idea. Here, the 
performance choice has put us in a situation where no one knows how many JavaFX 
apps have duplicate listener bugs, and such bugs can be very hard to debug.

Have anyone done any tests that prove that avoiding listener duplicates would 
lead to a severe performance impact?

I understand, that if all observables had thousands of listeners and we 
searched for a possible duplicate using an inefficient linear search, we would 
have a problem. But if reality is that very few observables have more than 20 
listeners and these could be stored in a map for efficient duplicate checking, 
then what is the problem?

Yours

Randahl


 
On 22 Jan 2014, at 11:26, Martin Sladecek martin.slade...@oracle.com wrote:

 The reason why this was decided this way is simple : performance. You usually 
 don't (try to) add a listener twice, so in most cases it doesn't make sense 
 to check for duplicates every time a listener is added. So we currently leave 
 the burden of avoiding duplicates on the developer.
 
 -Martin
 
 On 01/22/2014 11:23 AM, Randahl Fink Isaksen wrote:
 Hi Martin
 
 While I agree your proposed solution would work, I still don’t understand 
 why JavaFX should keep on supporting duplicates in listener collections. Can 
 anyone come up with just 1 example of an application that might be depending 
 on having two listeners on the same Observable? E.g. this kind of code:
 
 myObservable.addListener(myChangeListener); //add it
 myObservable.addListener(myChangeListener); //add it again
 
 In what kind of situation would this sort of code make any sense?
 
 If we all feel confident that the presence of duplicates listeners is always 
 an error, I warmly recommend changing the API to be duplicate free.
 
 Yours
 
 Randahl
 
 
 
 
 On 22 Jan 2014, at 11:07, Martin Sladecek martin.slade...@oracle.com wrote:
 
 Hi all,
 I would like to start discussion about an addition to API in Observable, 
 ObservableValue and all Observable collections.
 There were multiple requests for a way how to avoid duplicates in listeners 
 lists. The way RT-25613 solves this is that it introduces public boolean 
 hasListener(ListenerType listener) which would return true if the provided 
 listener is already registered.
 
 This has one significant drawback that all of Observable* are actually 
 interfaces. Means we can only add hasListener as a defender method. The 
 problem is with the default implementation. We cannot return anything 
 meaningful, so we have to throw an UnsupportedOperationException. The 
 problem is that this might blow up unexpectedly when some older 
 Observable implementation is used. Also, it might be easy to miss when 
 implementing the interface, since the IDE might not force you to implement 
 it.
 
 So as an alternative solution, I propose adding something like:
 
 ensureListener(ListenerType listener)
 
 which would make sure the listener is on the list and if a listener is 
 already present, the number of times listener is registered on the 
 Observable will NOT grow after this call.
 
 The default implementation (for Observable) would look like this:
 
 public default void ensureListener(InvalidationListener listener) {
removeListener(listener);
addListener(listener);
 }
 
 subclasses might do something more effective. The same would apply to 
 ObservableValue and ChangeListener and Observable[List|Set|Map] and 
 [List|Set|Map]ChangeListener.
 
 What do you think?
 
 JIRA link: https://javafx-jira.kenai.com/browse/RT-25613
 
 -Martin
 



Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method

2014-01-22 Thread Martin Sladecek

On 01/22/2014 11:27 AM, Tom Schindl wrote:

On 22.01.14 11:07, Martin Sladecek wrote:

Hi all,
I would like to start discussion about an addition to API in Observable,
ObservableValue and all Observable collections.
There were multiple requests for a way how to avoid duplicates in
listeners lists. The way RT-25613 solves this is that it introduces
public boolean hasListener(ListenerType listener) which would return
true if the provided listener is already registered.

This has one significant drawback that all of Observable* are actually
interfaces. Means we can only add hasListener as a defender method. The
problem is with the default implementation. We cannot return anything
meaningful, so we have to throw an UnsupportedOperationException. The
problem is that this might blow up unexpectedly when some older
Observable implementation is used. Also, it might be easy to miss when
implementing the interface, since the IDE might not force you to
implement it.

So as an alternative solution, I propose adding something like:

ensureListener(ListenerType listener)

which would make sure the listener is on the list and if a listener is
already present, the number of times listener is registered on the
Observable will NOT grow after this call.

The default implementation (for Observable) would look like this:

public default void ensureListener(InvalidationListener listener) {
 removeListener(listener);
 addListener(listener);
}

subclasses might do something more effective. The same would apply to
ObservableValue and ChangeListener and Observable[List|Set|Map] and
[List|Set|Map]ChangeListener.

Well this would destroy the order! I expect listeners to be called in
the correct order not? Why doing a remove and not simply check if the
listener has already been added?

Tom


Because there's no way to do it in the interface, hence the problem with 
hasListener default implementation.


Yes, the order would be broken, but it's actually not guaranteed. 
Although FX internally uses a List, 3rd party implementations of 
Observable or ObservableValue might use a Set for listeners for example. 
This was the idea, but funnily enough, the current Observable javadoc is 
quite strict on the duplicates which would rule out the Set:


/**
 * Adds an {@link InvalidationListener} which will be notified 
whenever the

 * {@code Observable} becomes invalid. If the same
 * listener is added more than once, then it will be notified more than
 * once. That is, no check is made to ensure uniqueness.

Personally, I would rather make this requirement less strict and allow 
both List backed and Set backed implementations.


-Martin


Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method

2014-01-22 Thread Martin Sladecek

On 01/22/2014 11:38 AM, Randahl Fink Isaksen wrote:

Hi Martin

Then I respectfully disagree with this design decision. In my point of view, 
choosing performance over ease of use is rarely a good idea. Here, the 
performance choice has put us in a situation where no one knows how many JavaFX 
apps have duplicate listener bugs, and such bugs can be very hard to debug.

Have anyone done any tests that prove that avoiding listener duplicates would 
lead to a severe performance impact?
I'm not the original author of the Observable API, but I remember there 
were many performance tests back at the days before 2.0 release, esp. 
when such API decision was made. So likely, the tests were done, but the 
results were not archived or anything.




I understand, that if all observables had thousands of listeners and we 
searched for a possible duplicate using an inefficient linear search, we would 
have a problem. But if reality is that very few observables have more than 20 
listeners and these could be stored in a map for efficient duplicate checking, 
then what is the problem?
Well it's always a balance between performance, dynamic footprint (a 
set/map for duplicates) and API. Having an API that extensively checks 
for all the mistakes a developer can do is an extreme case in the same 
manner as an API that does not check input at all. If a cleanup of some 
listeners is missing, it's a developer's bug (memory leak) no matter if 
there a duplicate listener check or not on the subsequent addListener 
call when the object becomes valid again (or whatnot).


I understand your position, there are APIs I would also like to behave 
differently, but the decision was already made, it's too late for the 
discussion. There might be 3rd implementations that use a list a don't 
guarantee a duplicate check, we don't want to make them suddenly broken.


I also disagree that such bugs are hard to debug, a simple printout 
should do.
In many cases it would likely just make a computations run more times 
than necessary. I can imagine doing a duplicate check on something like 
-Djavafx.debug=true.


Regards,
-Martin




Yours

Randahl


  
On 22 Jan 2014, at 11:26, Martin Sladecek martin.slade...@oracle.com wrote:



The reason why this was decided this way is simple : performance. You usually 
don't (try to) add a listener twice, so in most cases it doesn't make sense to 
check for duplicates every time a listener is added. So we currently leave the 
burden of avoiding duplicates on the developer.

-Martin

On 01/22/2014 11:23 AM, Randahl Fink Isaksen wrote:

Hi Martin

While I agree your proposed solution would work, I still don’t understand why 
JavaFX should keep on supporting duplicates in listener collections. Can anyone 
come up with just 1 example of an application that might be depending on having 
two listeners on the same Observable? E.g. this kind of code:

myObservable.addListener(myChangeListener); //add it
myObservable.addListener(myChangeListener); //add it again

In what kind of situation would this sort of code make any sense?

If we all feel confident that the presence of duplicates listeners is always an 
error, I warmly recommend changing the API to be duplicate free.

Yours

Randahl




On 22 Jan 2014, at 11:07, Martin Sladecek martin.slade...@oracle.com wrote:


Hi all,
I would like to start discussion about an addition to API in Observable, 
ObservableValue and all Observable collections.
There were multiple requests for a way how to avoid duplicates in listeners 
lists. The way RT-25613 solves this is that it introduces public boolean 
hasListener(ListenerType listener) which would return true if the provided 
listener is already registered.

This has one significant drawback that all of Observable* are actually interfaces. Means 
we can only add hasListener as a defender method. The problem is with the default 
implementation. We cannot return anything meaningful, so we have to throw an 
UnsupportedOperationException. The problem is that this might blow up unexpectedly when 
some older Observable implementation is used. Also, it might be easy to miss 
when implementing the interface, since the IDE might not force you to implement it.

So as an alternative solution, I propose adding something like:

ensureListener(ListenerType listener)

which would make sure the listener is on the list and if a listener is already 
present, the number of times listener is registered on the Observable will NOT 
grow after this call.

The default implementation (for Observable) would look like this:

public default void ensureListener(InvalidationListener listener) {
removeListener(listener);
addListener(listener);
}

subclasses might do something more effective. The same would apply to 
ObservableValue and ChangeListener and Observable[List|Set|Map] and 
[List|Set|Map]ChangeListener.

What do you think?

JIRA link: https://javafx-jira.kenai.com/browse/RT-25613

-Martin




Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method

2014-01-22 Thread Tomas Mikula
Hi Randahl,

I'm curious about an example where you would take advantage of the behavior
where multiple addListener(listener) calls add the listener just once.

Anyway, here [1] are helper classes InvalidationSubscriber and
ChangeSubscriber that allow you to do that:

InvalidationSubscriber subscriber = new InvalidationSubscriber(observable,
listener);

subscriber.subscribe(); // registers the listener
subscriber.subscribe(); // no-op

Cheers,
Tomas

[1] https://gist.github.com/TomasMikula/8557825

On Wed, Jan 22, 2014 at 11:23 AM, Randahl Fink Isaksen rand...@rockit.dkwrote:

 Hi Martin

 While I agree your proposed solution would work, I still don’t understand
 why JavaFX should keep on supporting duplicates in listener collections.
 Can anyone come up with just 1 example of an application that might be
 depending on having two listeners on the same Observable? E.g. this kind of
 code:

 myObservable.addListener(myChangeListener); //add it
 myObservable.addListener(myChangeListener); //add it again

 In what kind of situation would this sort of code make any sense?

 If we all feel confident that the presence of duplicates listeners is
 always an error, I warmly recommend changing the API to be duplicate free.

 Yours

 Randahl




 On 22 Jan 2014, at 11:07, Martin Sladecek martin.slade...@oracle.com
 wrote:

  Hi all,
  I would like to start discussion about an addition to API in Observable,
 ObservableValue and all Observable collections.
  There were multiple requests for a way how to avoid duplicates in
 listeners lists. The way RT-25613 solves this is that it introduces public
 boolean hasListener(ListenerType listener) which would return true if the
 provided listener is already registered.
 
  This has one significant drawback that all of Observable* are actually
 interfaces. Means we can only add hasListener as a defender method. The
 problem is with the default implementation. We cannot return anything
 meaningful, so we have to throw an UnsupportedOperationException. The
 problem is that this might blow up unexpectedly when some older
 Observable implementation is used. Also, it might be easy to miss when
 implementing the interface, since the IDE might not force you to implement
 it.
 
  So as an alternative solution, I propose adding something like:
 
  ensureListener(ListenerType listener)
 
  which would make sure the listener is on the list and if a listener is
 already present, the number of times listener is registered on the
 Observable will NOT grow after this call.
 
  The default implementation (for Observable) would look like this:
 
  public default void ensureListener(InvalidationListener listener) {
 removeListener(listener);
 addListener(listener);
  }
 
  subclasses might do something more effective. The same would apply to
 ObservableValue and ChangeListener and Observable[List|Set|Map] and
 [List|Set|Map]ChangeListener.
 
  What do you think?
 
  JIRA link: https://javafx-jira.kenai.com/browse/RT-25613
 
  -Martin




Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method

2014-01-22 Thread Tom Schindl
[...]

 Actually even when you would rely on the order, in situations when you
 call ensureListener, you don't really know if the listener is already
 there. So you might really expect that listener would be added at this
 point as the last. It's just that will always be the outcome if the
 default implementation is used.

It would be even worse if this would be different in none default
implementations.

Tom


Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method

2014-01-22 Thread Ali Ebrahimi
I suggest adding another overload for addListener method taking boolean
parameter  duplicateAllowed or duplicateNotAllowed.


On Wed, Jan 22, 2014 at 3:00 PM, Richard Bair richard.b...@oracle.comwrote:

  The default implementation (for Observable) would look like this:
 
  public default void ensureListener(InvalidationListener listener) {
 removeListener(listener);
 addListener(listener);
  }
 
  subclasses might do something more effective. The same would apply to
  ObservableValue and ChangeListener and Observable[List|Set|Map] and
  [List|Set|Map]ChangeListener.
 
  Well this would destroy the order! I expect listeners to be called in
  the correct order not?

 That’s a good point :-(

  Why doing a remove and not simply check if the
  listener has already been added?

 Because there is no way to check, except in the implementation. From the
 Observable interface level, there is no way to a) force all implementations
 of the interface to implement the method correctly (without breaking source
 compatibility anyway), or b) to provide a reasonable default implementation.

 Maybe this is one of those things we can’t fix on the Observable interface
 and just have to provide implementations of on our concrete properties.

 Richard


Move to JIRA [was: Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method]

2014-01-22 Thread Daniel Blaukopf
Hi Martin, Randahl, Tom, Richard, Tomas and Ali,

This is a productive discussion, but once we get to this level of detail JIRA 
is the place to have it, so that we don’t lose our record of it. Would you 
continue the discussion on https://javafx-jira.kenai.com/browse/RT-25613 ?

See 
https://wiki.openjdk.java.net/display/OpenJFX/Code+Reviews#CodeReviews-TechnicalDiscussionsandCodeReviews

Thanks,
Daniel

On Jan 22, 2014, at 7:23 PM, Stephen F Northover steve.x.northo...@oracle.com 
wrote:

 If we add this API, I like addListener(InvalidationListener, boolean) better 
 than ensureListener().
 
 Steve
 
 On 2014-01-22 8:20 AM, Ali Ebrahimi wrote:
 I suggest adding another overload for addListener method taking boolean
 parameter  duplicateAllowed or duplicateNotAllowed.
 
 
 On Wed, Jan 22, 2014 at 3:00 PM, Richard Bair richard.b...@oracle.comwrote:
 
 The default implementation (for Observable) would look like this:
 
 public default void ensureListener(InvalidationListener listener) {
removeListener(listener);
addListener(listener);
 }
 
 subclasses might do something more effective. The same would apply to
 ObservableValue and ChangeListener and Observable[List|Set|Map] and
 [List|Set|Map]ChangeListener.
 Well this would destroy the order! I expect listeners to be called in
 the correct order not?
 That’s a good point :-(
 
 Why doing a remove and not simply check if the
 listener has already been added?
 Because there is no way to check, except in the implementation. From the
 Observable interface level, there is no way to a) force all implementations
 of the interface to implement the method correctly (without breaking source
 compatibility anyway), or b) to provide a reasonable default implementation.
 
 Maybe this is one of those things we can’t fix on the Observable interface
 and just have to provide implementations of on our concrete properties.
 
 Richard
 



Re: Move to JIRA [was: Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method]

2014-01-22 Thread John Hendrikx
Unfortunately, discussing things in JIRA works very poorly and is a 
good way to end a productive discussion IMHO.  Mailinglists are much 
better suited to the task, as thousands of interesting mailinglists 
accross many developer communities will atest to.


Keeping a record is good, aren't these mailinglists archived?

--John

On 22/01/2014 18:47, Daniel Blaukopf wrote:

Hi Martin, Randahl, Tom, Richard, Tomas and Ali,

This is a productive discussion, but once we get to this level of detail JIRA 
is the place to have it, so that we don’t lose our record of it. Would you 
continue the discussion on https://javafx-jira.kenai.com/browse/RT-25613 ?

See 
https://wiki.openjdk.java.net/display/OpenJFX/Code+Reviews#CodeReviews-TechnicalDiscussionsandCodeReviews

Thanks,
Daniel

On Jan 22, 2014, at 7:23 PM, Stephen F Northoversteve.x.northo...@oracle.com  
wrote:


If we add this API, I like addListener(InvalidationListener, boolean) better 
than ensureListener().

Steve

On 2014-01-22 8:20 AM, Ali Ebrahimi wrote:

I suggest adding another overload for addListener method taking boolean
parameter  duplicateAllowed or duplicateNotAllowed.


On Wed, Jan 22, 2014 at 3:00 PM, Richard Bairrichard.b...@oracle.comwrote:


The default implementation (for Observable) would look like this:

public default void ensureListener(InvalidationListener listener) {
removeListener(listener);
addListener(listener);
}

subclasses might do something more effective. The same would apply to
ObservableValue and ChangeListener and Observable[List|Set|Map] and
[List|Set|Map]ChangeListener.

Well this would destroy the order! I expect listeners to be called in
the correct order not?

That’s a good point :-(


Why doing a remove and not simply check if the
listener has already been added?

Because there is no way to check, except in the implementation. From the
Observable interface level, there is no way to a) force all implementations
of the interface to implement the method correctly (without breaking source
compatibility anyway), or b) to provide a reasonable default implementation.

Maybe this is one of those things we can’t fix on the Observable interface
and just have to provide implementations of on our concrete properties.

Richard




Re: Move to JIRA [was: Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method]

2014-01-22 Thread Jonathan Giles
The point is that we'd rather have quick reference to the historical
discussion without having to cross-reference between jira and the
mailing list archives (which isn't automatically done). Once you add
yourself as a watcher to the jira issue you are notified of every
change, in much the same way as you are emailed of the discussion on
this mailing list.

A good case in point came up just today with Martin referencing
historical choices made with the Observable API, but not being able to
reference them as they are not recorded anywhere (and if they are, being
able to search for them is not trivial - the only hope is to find them
in your email client and then go off to the openjfx-dev archives and
manually search through the discussions until the right thread is
found). Whilst Jira search isn't great, it is better than this!

In other words, I can see an upside without any downside. What troubles
do you have with Jira specifically?

-- Jonathan

On 23/01/2014 11:29 a.m., John Hendrikx wrote:
 Unfortunately, discussing things in JIRA works very poorly and is a
 good way to end a productive discussion IMHO.  Mailinglists are much
 better suited to the task, as thousands of interesting mailinglists
 accross many developer communities will atest to.

 Keeping a record is good, aren't these mailinglists archived?

 --John

 On 22/01/2014 18:47, Daniel Blaukopf wrote:
 Hi Martin, Randahl, Tom, Richard, Tomas and Ali,

 This is a productive discussion, but once we get to this level of
 detail JIRA is the place to have it, so that we don’t lose our record
 of it. Would you continue the discussion on
 https://javafx-jira.kenai.com/browse/RT-25613 ?

 See
 https://wiki.openjdk.java.net/display/OpenJFX/Code+Reviews#CodeReviews-TechnicalDiscussionsandCodeReviews

 Thanks,
 Daniel

 On Jan 22, 2014, at 7:23 PM, Stephen F
 Northoversteve.x.northo...@oracle.com  wrote:

 If we add this API, I like addListener(InvalidationListener,
 boolean) better than ensureListener().

 Steve

 On 2014-01-22 8:20 AM, Ali Ebrahimi wrote:
 I suggest adding another overload for addListener method taking
 boolean
 parameter  duplicateAllowed or duplicateNotAllowed.


 On Wed, Jan 22, 2014 at 3:00 PM, Richard
 Bairrichard.b...@oracle.comwrote:

 The default implementation (for Observable) would look like this:

 public default void ensureListener(InvalidationListener listener) {
 removeListener(listener);
 addListener(listener);
 }

 subclasses might do something more effective. The same would
 apply to
 ObservableValue and ChangeListener and Observable[List|Set|Map] and
 [List|Set|Map]ChangeListener.
 Well this would destroy the order! I expect listeners to be
 called in
 the correct order not?
 That’s a good point :-(

 Why doing a remove and not simply check if the
 listener has already been added?
 Because there is no way to check, except in the implementation.
 From the
 Observable interface level, there is no way to a) force all
 implementations
 of the interface to implement the method correctly (without
 breaking source
 compatibility anyway), or b) to provide a reasonable default
 implementation.

 Maybe this is one of those things we can’t fix on the Observable
 interface
 and just have to provide implementations of on our concrete
 properties.

 Richard




Re: Move to JIRA [was: Re: [8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method]

2014-01-22 Thread Stephen F Northover

Hi John,

The goal is not to end the discussion!

It's a trade off.  Mailing lists are good because they provide a 
threaded discussion.  JIRA is bad because it is not threaded.  JIRA has 
the advantage that it captures data in a single place and provides a 
good history of why a decision was made.


There's no right answer here but the policy that the FX committers is 
using is to try to capture as much as possible in JIRA.


Steve

On 2014-01-22 5:29 PM, John Hendrikx wrote:
Unfortunately, discussing things in JIRA works very poorly and is a 
good way to end a productive discussion IMHO.  Mailinglists are much 
better suited to the task, as thousands of interesting mailinglists 
accross many developer communities will atest to.


Keeping a record is good, aren't these mailinglists archived?

--John

On 22/01/2014 18:47, Daniel Blaukopf wrote:

Hi Martin, Randahl, Tom, Richard, Tomas and Ali,

This is a productive discussion, but once we get to this level of 
detail JIRA is the place to have it, so that we don’t lose our record 
of it. Would you continue the discussion on 
https://javafx-jira.kenai.com/browse/RT-25613 ?


See 
https://wiki.openjdk.java.net/display/OpenJFX/Code+Reviews#CodeReviews-TechnicalDiscussionsandCodeReviews


Thanks,
Daniel

On Jan 22, 2014, at 7:23 PM, Stephen F 
Northoversteve.x.northo...@oracle.com  wrote:


If we add this API, I like addListener(InvalidationListener, 
boolean) better than ensureListener().


Steve

On 2014-01-22 8:20 AM, Ali Ebrahimi wrote:
I suggest adding another overload for addListener method taking 
boolean

parameter  duplicateAllowed or duplicateNotAllowed.


On Wed, Jan 22, 2014 at 3:00 PM, Richard 
Bairrichard.b...@oracle.comwrote:



The default implementation (for Observable) would look like this:

public default void ensureListener(InvalidationListener listener) {
removeListener(listener);
addListener(listener);
}

subclasses might do something more effective. The same would 
apply to

ObservableValue and ChangeListener and Observable[List|Set|Map] and
[List|Set|Map]ChangeListener.
Well this would destroy the order! I expect listeners to be 
called in

the correct order not?

That’s a good point :-(


Why doing a remove and not simply check if the
listener has already been added?
Because there is no way to check, except in the implementation. 
From the
Observable interface level, there is no way to a) force all 
implementations
of the interface to implement the method correctly (without 
breaking source
compatibility anyway), or b) to provide a reasonable default 
implementation.


Maybe this is one of those things we can’t fix on the Observable 
interface
and just have to provide implementations of on our concrete 
properties.


Richard