bearstampede / piccolo2d

Automatically exported from code.google.com/p/piccolo2d
0 stars 0 forks source link

Remove property code constants, add firePropertyChange(String, T, T) methods #198

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Remove all public PROPERTY_CODE_XXX and PROPERTY_XXX String constants, and 
refactor

firePropertyChange(int, String, Object, Object)

methods to

firePropertyChange(String, T, T)

where T is Object, int, boolean as in java.beans.PropertyChangeSupport.

I don't believe the performance reasons hinted at in the javadoc

http://www.piccolo2d.org/doc/piccolo2d.java/release-1.3/core/apidocs/edu/umd/cs/
piccolo/PNode.html#fireChildPropertyChange(java.beans.PropertyChangeEvent,%20int
)

are worth polluting the API with all these constant values, and convention in 
Swing programming is to use String literals for firePropertyChange, not public 
String constants.

Original issue reported on code.google.com by heue...@gmail.com on 22 Dec 2010 at 2:18

GoogleCodeExporter commented 9 years ago
I will commit this to svn trunk if there are no objections.

Original comment by heue...@gmail.com on 1 Nov 2011 at 7:25

GoogleCodeExporter commented 9 years ago
I feel uneasy about this...

Say I have a 1000 nodes at depth 100. (Say, a tall parent-child chain of a 100 
nodes and the bottom node has 1000 children.)

Now suppose I modify the 1000 children. Say I animate their offset/transform. 
Say the animation goes through 200 steps.

In the current version of Piccolo the parent node will get 0 events but with 
your proposal, now the parent gets 1000 * 200 events.

The performance of firing isn't costly, but clients will now see any event 
handlers fired unless each of their event handlers checks the source of the 
event to make sure it is just from the parent node.

That seems like quite a burden on the client, and quite a change in Piccolo 
behavior even for a major release.

The current default behavior is to not fire up the parent chain (seemingly 
desirable default behavior) unless you explicitly use 
setPropertyChangeParentMask.

Is my reasoning correct?

Original comment by atdi...@gmail.com on 2 Nov 2011 at 1:03

GoogleCodeExporter commented 9 years ago

Original comment by heue...@gmail.com on 15 Feb 2012 at 11:05