Name property for PNodes

[GoogleCodeExporter]

PSwing components need a simple name property in order to be easily
identified.  This is especially in the case of automated testing where you
want to be able to easily script a user interface by referencing nodes on
screen.

The proposal is to add a name property (with matching getters and setters)
to the PNode class.  This is a requirement for the FEST-Swing Piccolo
integration (separate enhancement).

Here is the code diff:
Index: src/edu/umd/cs/piccolo/PNode.java
===================================================================
--- src/edu/umd/cs/piccolo/PNode.java   (revision 20)
+++ src/edu/umd/cs/piccolo/PNode.java   (revision 22)
@@ -230,6 +230,8 @@
    private boolean childBoundsInvalid;
    private boolean occluded;

+    private String name;
+
     /**
     * Constructs a new PNode.
     * <P>
@@ -246,6 +248,19 @@
        visible = true;
    }

+    public PNode(String name) {
+        this();
+        this.name = name;
+   }
+
+    public String getName() {
+        return name;
+    }
+
+    public void setName(String name) {
+        this.name = name;
+    }
+
    //****************************************************************
    // Animation - Methods to animate this node.
    // 
@@ -3047,7 +3062,7 @@
     */
    protected String paramString() {
        StringBuffer result = new StringBuffer();
-       
+       result.append("name="+ (name == null ? "null" : name.toString()));
        result.append("bounds=" + (bounds == null ? "null" : bounds.toString()));
        result.append(",fullBounds=" + (fullBoundsCache == null ? "null" :
fullBoundsCache.toString()));
        result.append(",transform=" + (transform == null ? "null" :
transform.toString()));

Original issue reported on code.google.com by steveonjava on 24 Jun 2008 at 8:42

[GoogleCodeExporter]

Original comment by steveonjava on 25 Jun 2008 at 6:35

• Added labels: Milestone-1.3

[GoogleCodeExporter]

Comments from Michael:

The only one that raises flags for me is Issue #22.  The new name
property should be a bound property (in that it fires property change
events) and should be a final method.

Original comment by steveonjava on 26 Jun 2008 at 10:08

[GoogleCodeExporter]

Original comment by steveonjava on 2 Jul 2008 at 12:28

• Added labels: Milestone-2.0

[GoogleCodeExporter]

Original comment by steveonjava on 3 Jul 2008 at 5:05

• Added labels: Milestone-1.3

[GoogleCodeExporter]

Sounds like https://appframework.dev.java.net/intro/index.html
and
http://java.sun.com/j2se/1.5.0/docs/api/java/awt/Component.html#setName(java.lan
g.String)

+1 from my side.

But rather use a StringBuilder but StringBuffer and is the "null" magic really 
necessary?

Original comment by mr0...@mro.name on 4 Nov 2008 at 10:14

[GoogleCodeExporter]

and let's not mix "+" and "append" string concatenation.

Original comment by mr0...@mro.name on 5 Nov 2008 at 1:14

[GoogleCodeExporter]

null magic in paramString is necessary since PNode still has its default 
constructor.

Original comment by allain.lalonde on 13 Jul 2009 at 7:43

[GoogleCodeExporter]

Is the Constructor PNode(String name) necessary for FEST compliance?

Original comment by mr0...@mro.name on 23 Jul 2009 at 6:21

[GoogleCodeExporter]

looks like it's at least suggested: http://fest.easytesting.org/reflect/ 

BTW: I will restrict the name values to
http://www.w3.org/TR/SVGTiny12/struct.html#IDAttribute via a regexp match in the
setter (which will also be used from the ctor).

Also do this on Sat < noon UTC

Original comment by mr0...@mro.name on 23 Jul 2009 at 6:31

[GoogleCodeExporter]

Is it possible to use the PNode's client properties for FEST integration?  I 
would
rather not add stuff to PNode if not necessary.

http://www.piccolo2d.org/doc/piccolo2d.java/release-1.2.1/apidocs/edu/umd/cs/pic
colo/PNode.html#getAttribute%28java.lang.Object%29

Original comment by heue...@gmail.com on 23 Jul 2009 at 6:56

[GoogleCodeExporter]

it's the getName and setName methods that make the difference.

The interface expansion is sensible IMO.

Original comment by mr0...@mro.name on 23 Jul 2009 at 7:26

[GoogleCodeExporter]

I vote yes for this too. Might consider delegating getName and setName to the 
to the 
client property methods so that the actual size of a PNode in memory doesn't 
grown 
unless a name is actually set.

Original comment by allain.lalonde on 23 Jul 2009 at 7:52

[GoogleCodeExporter]

Delegating to client property methods is a fine idea.  That will fire
PROPERTY_CLIENT_PROPERTIES property change events instead of a PROPERTY_NAME 
change
event.  Is that ok?

Original comment by heue...@gmail.com on 23 Jul 2009 at 8:12

[GoogleCodeExporter]

using the client properties as backing storage makes the value check senseless. 
Also
opens the door to name clashes for this important property with special meaning.

So no, it's not a good idea. Also firing change events for name sound really 
odd to
me. A node's name shouldn't change during it's lifetime.

P.S.: I wished there was a name property a year ago, when I wanted to integrate 
with
https://appframework.dev.java.net/intro/ - it's Resource-Injection is based on a
"name" property...

Original comment by mr0...@mro.name on 23 Jul 2009 at 8:36

[GoogleCodeExporter]

Well, if it's a final field then there doesn't need to be a setter, and if it 
is not
final, then it should fire change events.

Is it possible to do this FEST and appframework stuff on PNodes, or just on 
PSwing
nodes?  Perhaps I would care less if these were added to PSwing instead.

Original comment by heue...@gmail.com on 24 Jul 2009 at 3:18

[GoogleCodeExporter]

I agree with Marcus' critique of delegating to client properties.

But I do think it should be possible to create a PNode with a name by using it's
default constructor and then calling it's setter. Especially in the case of a 
PNode
factory.

Original comment by allain.lalonde on 24 Jul 2009 at 3:21

[GoogleCodeExporter]

It's not about PSwing, it's about PNode.
It's not about Widgets.
I wouldn't make it final as to not require the ctor only to set it.
I would not hack it ontop a publicly writeable storage.
I would not trigger change events as to not encourage more-than-once change.
I also don't think the memory footprint of one more pointer or 2 methods really 
matter.

It's all about the name 'name'. Either add it and ease integration (FEST 
seemingly
and AppFramework for sure) or let it.

It's about the expansion of the public interface. So it's an important change.

I'll prepare the commit tomorrow, let's see what we do with it.

I'd also like to get a statement from Stephen why subclassing doesn't do.

Original comment by mr0...@mro.name on 24 Jul 2009 at 1:07

[GoogleCodeExporter]

implemented in r561 - have a look and vote please.

Original comment by mr0...@mro.name on 25 Jul 2009 at 1:12

[GoogleCodeExporter]

please verify & vote.

Original comment by mr0...@mro.name on 25 Jul 2009 at 1:15

• Changed state: Fixed

[GoogleCodeExporter]

needs more work. See discussion at r561

Original comment by mr0...@mro.name on 28 Jul 2009 at 9:47

• Changed state: Started

[GoogleCodeExporter]

undone in r597.

Original comment by mr0...@mro.name on 28 Jul 2009 at 7:14

[GoogleCodeExporter]

redone in r600. Please verify and vote.

Original comment by mr0...@mro.name on 28 Jul 2009 at 8:28

[GoogleCodeExporter]

Original comment by mr0...@mro.name on 28 Jul 2009 at 8:43

• Changed state: Fixed

[GoogleCodeExporter]

Original comment by allain.lalonde on 30 Oct 2009 at 3:48

• Changed state: Verified