You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by "Henning P. Schmiedehausen" <hp...@intermeta.de> on 2005/10/06 14:18:26 UTC

Re: svn commit: r295107 - in /jakarta/commons/proper/beanutils/trunk/src:

niallp@apache.org writes:

>     protected Object createProperty(String name, Class type) {
> 
>+        if (type == Object.class) {
>+            return null;
>+        }
>+

When I saw that checkin, I was wondering whether this is not just
something that plasters over the actual problem. As I can see,
currently passing a "null" value to a LDB results in
DynaClass.add(name) instead of DynaClass.add(name, type) being called
(LDB line 438), which in turn uses new DynaProperty(name), which then
is mapped inside the DynaProperty class to DynaProperty(name, Object.class).

How does this differ from set("foo", new Object()); ?

The use of "Object" here is a bit obfuscated IMHO. Maybe we should use
a "NullType" marker object?

	Best regards
		Henning



>         // Create Lists, arrays or DynaBeans
>         if (type.isArray() || List.class.isAssignableFrom(type)) {
>             return createIndexedProperty(name, type);
>@@ -728,7 +732,7 @@
>     }
> 
>     /**
>-     * Create a new Instance of a 'Primitive' Property
>+     * Create a new Instance of a <code>java.lang.Number</code> Property.
>      */
>     protected Object createNumberProperty(String name, Class type) {
> 
>@@ -737,13 +741,18 @@
>     }
> 
>     /**
>-     * Create a new Instance of a 'Mapped' Property
>+     * Create a new Instance of other Property types
>      */
>     protected Object createOtherProperty(String name, Class type) {
> 
>-        if (type == String.class || type == Boolean.class ||
>-            type == Character.class || Date.class.isAssignableFrom(type)) {
>+        if (type == Object.class    ||
>+            type == String.class    ||
>+            type == Boolean.class   ||
>+            type == Character.class ||
>+            Date.class.isAssignableFrom(type)) {
>+
>             return null;
>+
>         }
> 
>         try {

>Modified: jakarta/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/LazyDynaBeanTestCase.java
>URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/LazyDynaBeanTestCase.java?rev=295107&r1=295106&r2=295107&view=diff
>==============================================================================
>--- jakarta/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/LazyDynaBeanTestCase.java (original)
>+++ jakarta/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/LazyDynaBeanTestCase.java Wed Oct  5 13:35:31 2005
>@@ -116,6 +116,21 @@
>     }
> 
>     /**
>+     * Test Getting/Setting a 'null' Property
>+     */
>+    public void testNullProperty() {
>+
>+        // Check the property & value doesn't exist
>+        assertNull("Check Property doesn't exist", dynaClass.getDynaProperty(testProperty));
>+        assertNull("Check Value is null", bean.get(testProperty));
>+
>+        // Set a new property to null
>+        bean.set(testProperty, null);
>+        assertNull("Check Value is still null", bean.get(testProperty));
>+
>+    }
>+
>+    /**
>      * Test Setting a Simple Property when MutableDynaClass is set to restricted
>      */
>     public void testSimplePropertyRestricted() {



>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-dev-help@jakarta.apache.org

-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen          INTERMETA GmbH
hps@intermeta.de        +49 9131 50 654 0   http://www.intermeta.de/

RedHat Certified Engineer -- Jakarta Turbine Development  -- hero for hire
   Linux, Java, perl, Solaris -- Consulting, Training, Development

		      4 - 8 - 15 - 16 - 23 - 42

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


[BeanUtils] Re: svn commit: r295107 - in /jakarta/commons/proper/beanutils/trunk/src:

Posted by Niall Pemberton <ni...@blueyonder.co.uk>.
From: "Henning P. Schmiedehausen" <hp...@intermeta.de>

> niallp@apache.org writes:
>
> >     protected Object createProperty(String name, Class type) {
> >
> >+        if (type == Object.class) {
> >+            return null;
> >+        }
> >+
>
> When I saw that checkin, I was wondering whether this is not just
> something that plasters over the actual problem. As I can see,
> currently passing a "null" value to a LDB results in
> DynaClass.add(name) instead of DynaClass.add(name, type) being called
> (LDB line 438), which in turn uses new DynaProperty(name), which then
> is mapped inside the DynaProperty class to DynaProperty(name,
Object.class).

Yes this is true. If the first time a property's value is set it is null
then its a  problem because there is no way of determining the type. I did
consider ignoring them and not automatically adding the DynaProperty in that
case - but then this property would be missing when someone processes the
properties of a DynaClass/DynaBean.

Originally one aim of LDB was to automatically instantiate POPJO Bean (or
POJO Bean array) properties - the problem is, there is no way of identifying
when an object is a "bean" as oppose to something else - so it achieves that
effectively by saying well if its not a "DynaBean, Map, List, array,
primitive, Number, String or Date" then I'm going to try and instantiate it.
I believe now that this aim was too optimistic and LDB shouldn't have tried
to achieve that - since it leaves alot of property types that it will
automatically instantiate that you probably don't want it to (including
those of type java.lang.Object). The problem is, since its been released I
don't want to screw up people relying on the current behaviour by taking
whats in createOtherProperty out. Personally I have my own LDB
implementation that does just that - by overriding the createOtherProperty()
method to just return null.

However I think duplicating the check for java.lang.Object types in this
change [at the start of the createProperty() method that you're highlighting
and in the createOtherProperty() method] was a mistake so I've removed the
one from the createProperty() method - that way people just have to override
the createOtherProperty() method to implement alternative behaviour.

> How does this differ from set("foo", new Object()); ?

Well here you're explicity setting a property value to java.lang.Object -
rather than LDB automatically instantiating a java.lang.Object for you.

> The use of "Object" here is a bit obfuscated IMHO. Maybe we should use
> a "NullType" marker object?


I did ponder using an "UndefinedType" for a DynaProperty that was
automatically added via set("foo" null) - but that led me to think that if a
"none null" value was later set should I replace the DynaProperty with
"UndefinedType" with one with the class of the new value? That seemed to
open up a more complex route (modifying DynaProperty's), so it seemed best
to keep it simple and leave it as it was. Also,  an "UndefinedType" creates
a headache for someone retrieveing and processing the properties of the
DynaClass.

IMO there isn't an ideal route for properties that are first set with a
value of "null" and so would probably favour not doing anything more - if
someone wants to avoid this happening, then all they need do is add a
DynaProperty with the appropriate type to the LazyDynaClass first.

Thanks for your comments.

Niall

> Best regards
> Henning



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org