You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@ofbiz.apache.org by le...@apache.org on 2009/11/24 08:43:19 UTC

svn commit: r883613 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java

Author: lektran
Date: Tue Nov 24 07:43:18 2009
New Revision: 883613

URL: http://svn.apache.org/viewvc?rev=883613&view=rev
Log:
Revert Jacques changes to EntitySaxReader.java from r883549 and 883507, the UtilValidate.is(Not)Empty methods are not setup to test CharSequence instances.  Hopefully there aren't too many more out there.

Modified:
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java?rev=883613&r1=883612&r2=883613&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java Tue Nov 24 07:43:18 2009
@@ -375,7 +375,7 @@
 
         if (currentValue != null) {
             if (currentFieldName != null) {
-                if (UtilValidate.isNotEmpty(currentFieldValue)) {
+                if (currentFieldValue != null && currentFieldValue.length() > 0) {
                     if (currentValue.getModelEntity().isField(currentFieldName.toString())) {
                         ModelEntity modelEntity = currentValue.getModelEntity();
                         ModelField modelField = modelEntity.getField(currentFieldName.toString());
@@ -499,7 +499,7 @@
                 CharSequence name = attributes.getLocalName(i);
                 CharSequence value = attributes.getValue(i);
 
-                if (UtilValidate.isEmpty(name)) {
+                if (name == null || name.length() == 0) {
                     name = attributes.getQName(i);
                 }
                 newElement.setAttribute(name.toString(), value.toString());
@@ -548,12 +548,12 @@
                     CharSequence name = attributes.getLocalName(i);
                     CharSequence value = attributes.getValue(i);
 
-                    if (UtilValidate.isEmpty(name)) {
+                    if (name == null || name.length() == 0) {
                         name = attributes.getQName(i);
                     }
                     try {
                         // treat empty strings as nulls
-                        if (UtilValidate.isNotEmpty(value)) {
+                        if (value != null && value.length() > 0) {
                             if (currentValue.getModelEntity().isField(name.toString())) {
                                 currentValue.setString(name.toString(), value.toString());
                             } else {

Re: svn commit: r883613 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java

Posted by Adam Heath <do...@brainfood.com>.

Tim Ruppert wrote:
> Everybody's got their own style dude - I could find you 10 articles that
> will flip it.  Besides - there are no absolutes in this world, so I'll
> refrain from spending my day trying to prove that the instance that you
> were referring to IS a bad practice.
> 
> When writing object oriented code I also think it's generally a "bad"
> practice to just use Object, but instead tend to not use that catch all
> as the way I write my code - but hey that's just me.  I think better
> thought thru designs yield cleaner interfaces that doing things like
> "instanceof" and setting everything to object.
> 
> Just my two cents - feel free to drop them on the floor (face up) for
> the next lucky person walking by :)

You said "instance of is bad practice."  That means you think it is
wrong in almost all situations.  And that is what I was commenting about.

Using Object for types is a side-effect of ofbiz's tendency to use
Maps for tons of things.  So we *have* to do this kind of run-time
object testing.

Using Object is also useful when you want to support graceful upgrades
of a low-level object to some higher level.  Like auto-converting
String to a Number.

Re: svn commit: r883613 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java

Posted by Tim Ruppert <ti...@hotwaxmedia.com>.

Thanks for the clear explanation Adrian.  Instanceof was put into the  
language because there are times when it needs to be used - and it  
sounds like we're using it.  I'd have to look at the specific lines to  
know if I could clean it up more to have methods and interface  
implementations that know better how to handle the appropriate type of  
objects  - but in this case it seems like the simplest thing that will  
work.

Again, thanks for the heads up - much appreciated.

Cheers,
Ruppert
--
Tim Ruppert
HotWax Media
http://www.hotwaxmedia.com

o:801.649.6594
f:801.649.6595

On Nov 24, 2009, at 12:01 PM, Adrian Crum wrote:

> I think for the most part we're following good design principals.  
> The use of instanceof in this case is because we don't know the  
> object's type at compile time, and that happens often in the script  
> interpreter code.
>
> Take the new converter code as an example. It uses Gang of Four  
> design patterns and one of Martin Fowler's refactoring methods, yet  
> it still needs to use instanceof. If you have Object A that is an  
> unknown type, and you need to convert it to Object type B, you have  
> to use instanceof to determine the type of Object A in order to  
> lookup the correct converter.
>
> -Adrian
>
>
> Tim Ruppert wrote:
>> Everybody's got their own style dude - I could find you 10 articles  
>> that will flip it.  Besides - there are no absolutes in this world,  
>> so I'll refrain from spending my day trying to prove that the  
>> instance that you were referring to IS a bad practice.
>> When writing object oriented code I also think it's generally a  
>> "bad" practice to just use Object, but instead tend to not use that  
>> catch all as the way I write my code - but hey that's just me.  I  
>> think better thought thru designs yield cleaner interfaces that  
>> doing things like "instanceof" and setting everything to object.
>> Just my two cents - feel free to drop them on the floor (face up)  
>> for the next lucky person walking by :)
>> Cheers,
>> Ruppert
>> -- 
>> Tim Ruppert
>> HotWax Media
>> http://www.hotwaxmedia.com
>> o:801.649.6594
>> f:801.649.6595
>> On Nov 24, 2009, at 11:09 AM, Adam Heath wrote:
>>> Tim Ruppert wrote:
>>>> Instance of is a bad practice to get into in object oriented code -
>>>> might as well just go back to C and switch statements.
>>>
>>> ==
>>> String foo = (String) context.get("foo");
>>>
>>> if (UtilValidate.isEmpty(foo)) {
>>> }
>>>
>>> Object bar = context.get("bar");
>>> Map barMap;
>>> if (UtilValidate.isEmpty(bar)) {
>>>    return;
>>> } else if (bar instanceof String) {
>>>    barMap = parseStringAsJSONMap((String) bar);
>>> } else if (bar instanceof Collection) {
>>>    barMap = convertCollectionToMap((Collection) bar);
>>> } else if (bar instanceof Map) {
>>>    barMap = (Map) bar;
>>> } else {
>>>    throw new IllegalArgumentException("Can't handle bar");
>>> }
>>> ==
>>>
>>> The first isEmpty call calls isEmpty(String).  The latter calls
>>> isEmpty(Object).  The latter method then needs to do the instanceof
>>> itself.  But I assume you understand that.
>>>
>>> Besides, you are deep into the 'instanceof always considered bad
>>> camp'.  This is wrong.  See
>>>
>>> http://www.velocityreviews.com/forums/t302491-instanceof-not-always-bad-the-instanceof-myth.html
>>>
>>>
>>>
>>>
>>>
>>>
>>>>
>>>> Cheers,
>>>> Ruppert
>>>>
>>>> On Nov 24, 2009, at 8:45 AM, Adam Heath wrote:
>>>>
>>>>> Adrian Crum wrote:
>>>>>> Or better yet, overload the method so you don't need all those
>>>>>> conditionals.
>>>>>
>>>>> If the type of the object in the calling method is Object, then it
>>>>> won't call the overloaded method.
>>>>>
>>>>> However, this is better:
>>>>>
>>>>> if (object instanceof String) {
>>>>>   return UtilValidate.isEmpty((String) object);
>>>>> }
>>>>
>>>

Re: svn commit: r883613 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java

Posted by Adrian Crum <ad...@hlmksw.com>.

I think for the most part we're following good design principals. The 
use of instanceof in this case is because we don't know the object's 
type at compile time, and that happens often in the script interpreter code.

Take the new converter code as an example. It uses Gang of Four design 
patterns and one of Martin Fowler's refactoring methods, yet it still 
needs to use instanceof. If you have Object A that is an unknown type, 
and you need to convert it to Object type B, you have to use instanceof 
to determine the type of Object A in order to lookup the correct converter.

-Adrian


Tim Ruppert wrote:
> Everybody's got their own style dude - I could find you 10 articles that 
> will flip it.  Besides - there are no absolutes in this world, so I'll 
> refrain from spending my day trying to prove that the instance that you 
> were referring to IS a bad practice.
> 
> When writing object oriented code I also think it's generally a "bad" 
> practice to just use Object, but instead tend to not use that catch all 
> as the way I write my code - but hey that's just me.  I think better 
> thought thru designs yield cleaner interfaces that doing things like 
> "instanceof" and setting everything to object.
> 
> Just my two cents - feel free to drop them on the floor (face up) for 
> the next lucky person walking by :)
> 
> Cheers,
> Ruppert
> -- 
> Tim Ruppert
> HotWax Media
> http://www.hotwaxmedia.com
> 
> o:801.649.6594
> f:801.649.6595
> 
> On Nov 24, 2009, at 11:09 AM, Adam Heath wrote:
> 
>> Tim Ruppert wrote:
>>> Instance of is a bad practice to get into in object oriented code -
>>> might as well just go back to C and switch statements.
>>
>> ==
>> String foo = (String) context.get("foo");
>>
>> if (UtilValidate.isEmpty(foo)) {
>> }
>>
>> Object bar = context.get("bar");
>> Map barMap;
>> if (UtilValidate.isEmpty(bar)) {
>>     return;
>> } else if (bar instanceof String) {
>>     barMap = parseStringAsJSONMap((String) bar);
>> } else if (bar instanceof Collection) {
>>     barMap = convertCollectionToMap((Collection) bar);
>> } else if (bar instanceof Map) {
>>     barMap = (Map) bar;
>> } else {
>>     throw new IllegalArgumentException("Can't handle bar");
>> }
>> ==
>>
>> The first isEmpty call calls isEmpty(String).  The latter calls
>> isEmpty(Object).  The latter method then needs to do the instanceof
>> itself.  But I assume you understand that.
>>
>> Besides, you are deep into the 'instanceof always considered bad
>> camp'.  This is wrong.  See
>>
>> http://www.velocityreviews.com/forums/t302491-instanceof-not-always-bad-the-instanceof-myth.html 
>>
>>
>>
>>
>>
>>
>>
>>>
>>> Cheers,
>>> Ruppert
>>>
>>> On Nov 24, 2009, at 8:45 AM, Adam Heath wrote:
>>>
>>>> Adrian Crum wrote:
>>>>> Or better yet, overload the method so you don't need all those
>>>>> conditionals.
>>>>
>>>> If the type of the object in the calling method is Object, then it
>>>> won't call the overloaded method.
>>>>
>>>> However, this is better:
>>>>
>>>> if (object instanceof String) {
>>>>    return UtilValidate.isEmpty((String) object);
>>>> }
>>>
>>
>

Re: svn commit: r883613 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java

Posted by Tim Ruppert <ti...@hotwaxmedia.com>.

Everybody's got their own style dude - I could find you 10 articles  
that will flip it.  Besides - there are no absolutes in this world, so  
I'll refrain from spending my day trying to prove that the instance  
that you were referring to IS a bad practice.

When writing object oriented code I also think it's generally a "bad"  
practice to just use Object, but instead tend to not use that catch  
all as the way I write my code - but hey that's just me.  I think  
better thought thru designs yield cleaner interfaces that doing things  
like "instanceof" and setting everything to object.

Just my two cents - feel free to drop them on the floor (face up) for  
the next lucky person walking by :)

Cheers,
Ruppert
--
Tim Ruppert
HotWax Media
http://www.hotwaxmedia.com

o:801.649.6594
f:801.649.6595

On Nov 24, 2009, at 11:09 AM, Adam Heath wrote:

> Tim Ruppert wrote:
>> Instance of is a bad practice to get into in object oriented code -
>> might as well just go back to C and switch statements.
>
> ==
> String foo = (String) context.get("foo");
>
> if (UtilValidate.isEmpty(foo)) {
> }
>
> Object bar = context.get("bar");
> Map barMap;
> if (UtilValidate.isEmpty(bar)) {
> 	return;
> } else if (bar instanceof String) {
> 	barMap = parseStringAsJSONMap((String) bar);
> } else if (bar instanceof Collection) {
> 	barMap = convertCollectionToMap((Collection) bar);
> } else if (bar instanceof Map) {
> 	barMap = (Map) bar;
> } else {
> 	throw new IllegalArgumentException("Can't handle bar");
> }
> ==
>
> The first isEmpty call calls isEmpty(String).  The latter calls
> isEmpty(Object).  The latter method then needs to do the instanceof
> itself.  But I assume you understand that.
>
> Besides, you are deep into the 'instanceof always considered bad
> camp'.  This is wrong.  See
>
> http://www.velocityreviews.com/forums/t302491-instanceof-not-always-bad-the-instanceof-myth.html
>
>
>
>
>
>
>>
>> Cheers,
>> Ruppert
>>
>> On Nov 24, 2009, at 8:45 AM, Adam Heath wrote:
>>
>>> Adrian Crum wrote:
>>>> Or better yet, overload the method so you don't need all those
>>>> conditionals.
>>>
>>> If the type of the object in the calling method is Object, then it
>>> won't call the overloaded method.
>>>
>>> However, this is better:
>>>
>>> if (object instanceof String) {
>>>    return UtilValidate.isEmpty((String) object);
>>> }
>>
>

Re: svn commit: r883613 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java

Posted by Adam Heath <do...@brainfood.com>.

Tim Ruppert wrote:
> Instance of is a bad practice to get into in object oriented code -
> might as well just go back to C and switch statements.

==
String foo = (String) context.get("foo");

if (UtilValidate.isEmpty(foo)) {
}

Object bar = context.get("bar");
Map barMap;
if (UtilValidate.isEmpty(bar)) {
	return;
} else if (bar instanceof String) {
	barMap = parseStringAsJSONMap((String) bar);
} else if (bar instanceof Collection) {
	barMap = convertCollectionToMap((Collection) bar);
} else if (bar instanceof Map) {
	barMap = (Map) bar;
} else {
	throw new IllegalArgumentException("Can't handle bar");
}
==

The first isEmpty call calls isEmpty(String).  The latter calls
isEmpty(Object).  The latter method then needs to do the instanceof
itself.  But I assume you understand that.

Besides, you are deep into the 'instanceof always considered bad
camp'.  This is wrong.  See

http://www.velocityreviews.com/forums/t302491-instanceof-not-always-bad-the-instanceof-myth.html






> 
> Cheers,
> Ruppert
> 
> On Nov 24, 2009, at 8:45 AM, Adam Heath wrote:
> 
>> Adrian Crum wrote:
>>> Or better yet, overload the method so you don't need all those
>>> conditionals.
>>
>> If the type of the object in the calling method is Object, then it
>> won't call the overloaded method.
>>
>> However, this is better:
>>
>> if (object instanceof String) {
>>     return UtilValidate.isEmpty((String) object);
>> }
>

Re: svn commit: r883613 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java

Posted by Tim Ruppert <ti...@hotwaxmedia.com>.

Instance of is a bad practice to get into in object oriented code -  
might as well just go back to C and switch statements.

Cheers,
Ruppert

On Nov 24, 2009, at 8:45 AM, Adam Heath wrote:

> Adrian Crum wrote:
>> Or better yet, overload the method so you don't need all those
>> conditionals.
>
> If the type of the object in the calling method is Object, then it
> won't call the overloaded method.
>
> However, this is better:
>
> if (object instanceof String) {
> 	return UtilValidate.isEmpty((String) object);
> }

Re: svn commit: r883613 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java

Posted by Adam Heath <do...@brainfood.com>.

Adrian Crum wrote:
> Or better yet, overload the method so you don't need all those
> conditionals.

If the type of the object in the calling method is Object, then it
won't call the overloaded method.

However, this is better:

if (object instanceof String) {
	return UtilValidate.isEmpty((String) object);
}

Re: svn commit: r883613 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java

Posted by Adrian Crum <ad...@hlmksw.com>.

Or better yet, overload the method so you don't need all those conditionals.

-Adrian

Scott Gray wrote:
> Good point, but it would be better to do this:
> if (...) {
> } else if (...) {
> } else if (...) {
> } else {
>     Debug.logWarning(...);
> }
> return false;
> 
> And about changing:
> if (value == null) return true;
> personally, I don't have a problem with (and actually prefer) using a 
> single line for simple statements.
> 
> Regards
> Scott
> 
> On 25/11/2009, at 12:06 AM, Jacques Le Roux wrote:
> 
>> I see your point now (that's why I asked was not sure what you 
>> meaned). But we need to differentiate known from unknown, so I propose 
>> rather
>>
>> Index: framework/base/src/org/ofbiz/base/util/ObjectType.java
>> ===================================================================
>> --- framework/base/src/org/ofbiz/base/util/ObjectType.java (revision 
>> 883647)
>> +++ framework/base/src/org/ofbiz/base/util/ObjectType.java (working copy)
>> @@ -754,25 +754,36 @@
>>    }
>>
>>    public static boolean isEmpty(Object value) {
>> -        if (value == null) return true;
>> +        if (value == null) {
>> +            return true;
>> +        }
>>
>>        if (value instanceof String) {
>>            if (((String) value).length() == 0) {
>>                return true;
>> +            } else {
>> +         return false;
>>            }
>>        } else if (value instanceof Collection) {
>>            if (((Collection<?>) value).size() == 0) {
>>                return true;
>> +            } else {
>> +         return false;
>>            }
>>        } else if (value instanceof Map) {
>>            if (((Map<?,?>) value).size() == 0) {
>>                return true;
>> +            } else {
>> +         return false;
>>            }
>>        } else if (value instanceof CharSequence) {
>>            if (((CharSequence) value).length() == 0) {
>>                return true;
>> +            } else {
>> +         return false;
>>            }
>>        }
>> +        Debug.logWarning("In ObjectType.isEmpty(Object) returning 
>> false for unknown not-null Object.", module);
>>        return false;
>>    }
>>
>> Jacques
>>
>> ----- Original Message ----- From: "Scott Gray" 
>> <sc...@hotwaxmedia.com>
>> To: <de...@ofbiz.apache.org>
>> Sent: Tuesday, November 24, 2009 11:44 AM
>> Subject: Re: svn commit: r883613 - 
>> /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java 
>>
>>
>>
>>> Wrong spot, it needs to go just before the end of the method:
>>> Debug.logWarning("In ObjectType.isEmpty(Object) returning false for  
>>> unknown not-null Object.");
>>> return false;
>>>
>>> Regards
>>> Scott
>>>
>>> On 24/11/2009, at 11:33 PM, Jacques Le Roux wrote:
>>>
>>>> Hi Scott,
>>>>
>>>> What do you think about adding this check at the beginning of  
>>>> ObjectType.isEmpty()  something like
>>>>
>>>> Index: framework/base/src/org/ofbiz/base/util/ObjectType.java
>>>> ===================================================================
>>>> --- framework/base/src/org/ofbiz/base/util/ObjectType.java 
>>>> (revision  883647)
>>>> +++ framework/base/src/org/ofbiz/base/util/ObjectType.java (working  
>>>> copy)
>>>> @@ -754,7 +754,10 @@
>>>>   }
>>>>
>>>>   public static boolean isEmpty(Object value) {
>>>> -        if (value == null) return true;
>>>> +        if (value == null) {
>>>> +            Debug.logWarning("a warning", module);
>>>> +            return true;
>>>> +        }
>>>>
>>>> Jacques
>>>>
>>>>
>>>> From: "Scott Gray" <sc...@hotwaxmedia.com>
>>>>> Thanks Jacques, it wasn't a compilation problem but a run-install   
>>>>> problem, empty fields were being treated as empty strings instead  
>>>>> of  null which was causing all sorts of fk problems during data load.
>>>>>
>>>>> If you do work on UtilValidate it might be a good idea to log a   
>>>>> warning whenever an object is passed in that we can only do a 
>>>>> null   check on, it might help people avoid this sort of issue and 
>>>>> help  us  identify any areas where the checks aren't doing what we  
>>>>> probably  intended them to do.
>>>>>
>>>>> Regards
>>>>> Scott
>>>>>
>>>>> On 24/11/2009, at 9:49 PM, Jacques Le Roux wrote:
>>>>>
>>>>>> Thanks Scott,
>>>>>>
>>>>>> I did not notice build issues. I will double check possible   
>>>>>> CharSequence uses or rather coonsider implementing
>>>>>> UtilValidate.is(Not)Empty for it (I can't see any reason to not  
>>>>>> do  it at this stage)
>>>>>>
>>>>>> Jacques
>>>>>>
>>>>>> From: <le...@apache.org>
>>>>>>> Author: lektran
>>>>>>> Date: Tue Nov 24 07:43:18 2009
>>>>>>> New Revision: 883613
>>>>>>>
>>>>>>> URL: http://svn.apache.org/viewvc?rev=883613&view=rev
>>>>>>> Log:
>>>>>>> Revert Jacques changes to EntitySaxReader.java from r883549 and   
>>>>>>> 883507, the UtilValidate.is(Not)Empty methods are not setup to
>>>>>>> test CharSequence instances.  Hopefully there aren't too many  
>>>>>>> more  out there.
>>>>>>>
>>>>>>> Modified:
>>>>>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/  
>>>>>>> EntitySaxReader.java
>>>>>>>
>>>>>>> Modified: 
>>>>>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/  
>>>>>>> EntitySaxReader.java
>>>>>>> URL:
>>>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java?rev=883613&r1=883612&r2=883613&view=diff 
>>>>>>>
>>>>>>> = = = = = = = = =  = = 
>>>>>>> ===================================================================
>>>>>>> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/  
>>>>>>> EntitySaxReader.java (original)
>>>>>>> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/  
>>>>>>> EntitySaxReader.java Tue Nov 24 07:43:18 2009
>>>>>>> @@ -375,7 +375,7 @@
>>>>>>>
>>>>>>>      if (currentValue != null) {
>>>>>>>          if (currentFieldName != null) {
>>>>>>> -                if (UtilValidate.isNotEmpty(currentFieldValue)) {
>>>>>>> +                if (currentFieldValue != null &&   
>>>>>>> currentFieldValue.length() > 0) {
>>>>>>>                  if   
>>>>>>> (currentValue .getModelEntity().isField(currentFieldName.toString())) 
>>>>>>> {
>>>>>>>                      ModelEntity modelEntity =   
>>>>>>> currentValue.getModelEntity();
>>>>>>>                      ModelField modelField =   
>>>>>>> modelEntity.getField(currentFieldName.toString());
>>>>>>> @@ -499,7 +499,7 @@
>>>>>>>              CharSequence name = attributes.getLocalName(i);
>>>>>>>              CharSequence value = attributes.getValue(i);
>>>>>>>
>>>>>>> -                if (UtilValidate.isEmpty(name)) {
>>>>>>> +                if (name == null || name.length() == 0) {
>>>>>>>                  name = attributes.getQName(i);
>>>>>>>              }
>>>>>>>              newElement.setAttribute(name.toString(),   
>>>>>>> value.toString());
>>>>>>> @@ -548,12 +548,12 @@
>>>>>>>                  CharSequence name = attributes.getLocalName(i);
>>>>>>>                  CharSequence value = attributes.getValue(i);
>>>>>>>
>>>>>>> -                    if (UtilValidate.isEmpty(name)) {
>>>>>>> +                    if (name == null || name.length() == 0) {
>>>>>>>                      name = attributes.getQName(i);
>>>>>>>                  }
>>>>>>>                  try {
>>>>>>>                      // treat empty strings as nulls
>>>>>>> -                        if (UtilValidate.isNotEmpty(value)) {
>>>>>>> +                        if (value != null && value.length() > 0) {
>>>>>>>                          if   
>>>>>>> (currentValue.getModelEntity().isField(name.toString())) {
>>>>>>>                                
>>>>>>> currentValue.setString(name.toString(), value.toString());
>>>>>>>                          } else {
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>