You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Joerg Heinicke <jo...@gmx.de> on 2004/03/03 20:55:40 UTC

Re: cvs commit: cocoon-2.1/src/blocks/woody/java/org/apache/cocoon/woody/binding RepeaterJXPathBinding.java

On 03.03.2004 20:47, joerg@apache.org wrote:
> joerg       2004/03/03 11:47:35
> 
>   Modified:    src/blocks/woody/java/org/apache/cocoon/woody/binding
>                         RepeaterJXPathBinding.java
>   Log:
>   clean up: removed unused code (for reverting changes we have CVS, so please remove old stuff always), JavaDoc added, comments fixed;
> 
>   changed isNullAllListElements() => isAnyListElementNotNull(): the duplicate negation at usage time breaks my brain ;-)

For one case there is now another behaviour: if the list is empty now 
false will be returned (as before, but there the function tested the 
opposite). I don't know if this breaks anything, it was just for logical 
reasons:
isNullAllListElements() returning false I expect to get one list element 
which is not null, which is not the case for empty list.
Antonio, I guess you are the only one using it at the moment. Can you 
test it please?

Joerg

Re: cvs commit: cocoon-2.1/src/blocks/woody/java/org/apache/cocoon/woody/bindingRepeat erJXPathBinding.java

Posted by Antonio Gallardo <ag...@agssa.net>.
Joerg Heinicke dijo:
> On 04.03.2004 03:06, Antonio Gallardo wrote:
>
>>>Antonio, I guess you are the only one using it at the moment. Can you
>>>test it please?
>>
>> I tested it before committing. I can not assure it is totaly bug free.
>> What I can said is I will use it in many places. If there is a bug it
>> will
>> be showed.
>
> It was not meant that I expect your code to be bug free. I just thought
> you already have it in work somewhere and after my changes that I made
> more or less just for personal taste and consistency I asked you for
> testing it again, so that I have the comparison between your and my
> changes and no regression came in.

Yep. I did it. The code is working good. Thanks.

I thought you jut changed the internal names and I answer to this mail
before reviewing the posted code. Later, I saw you make a full refactoring
instead of just changing some internal names.

Best Regards,

Antonio Gallardo

Re: cvs commit: cocoon-2.1/src/blocks/woody/java/org/apache/cocoon/woody/bindingRepeat erJXPathBinding.java

Posted by Joerg Heinicke <jo...@gmx.de>.
On 04.03.2004 03:06, Antonio Gallardo wrote:

>>Antonio, I guess you are the only one using it at the moment. Can you
>>test it please?
> 
> I tested it before committing. I can not assure it is totaly bug free.
> What I can said is I will use it in many places. If there is a bug it will
> be showed.

It was not meant that I expect your code to be bug free. I just thought 
you already have it in work somewhere and after my changes that I made 
more or less just for personal taste and consistency I asked you for 
testing it again, so that I have the comparison between your and my 
changes and no regression came in.

Joerg

Re: cvs commit: cocoon-2.1/src/blocks/woody/java/org/apache/cocoon/woody/bindingRepeat erJXPathBinding.java

Posted by Antonio Gallardo <ag...@agssa.net>.
Joerg Heinicke dijo:
> On 03.03.2004 20:47, joerg@apache.org wrote:
>> joerg       2004/03/03 11:47:35
>>
>>   Modified:    src/blocks/woody/java/org/apache/cocoon/woody/binding
>>                         RepeaterJXPathBinding.java
>>   Log:
>>   clean up: removed unused code (for reverting changes we have CVS, so
>> please remove old stuff always), JavaDoc added, comments fixed;
>>
>>   changed isNullAllListElements() => isAnyListElementNotNull(): the
>> duplicate negation at usage time breaks my brain ;-)
>
> For one case there is now another behaviour: if the list is empty now
> false will be returned (as before, but there the function tested the
> opposite). I don't know if this breaks anything, it was just for logical
> reasons:
> isNullAllListElements() returning false I expect to get one list element
> which is not null, which is not the case for empty list.
> Antonio, I guess you are the only one using it at the moment. Can you
> test it please?

I tested it before committing. I can not assure it is totaly bug free.
What I can said is I will use it in many places. If there is a bug it will
be showed.

By definition , an empty List is not allowed. If the
RepeaterJXPathBindingBuilder detect there not exist any definition of
unique-row in the old and new style it throws an exception. This is the
same as it did before.

isNullAllListElements() - This method return true is all the elements
inside the List points to null (This is similar as it was tested before).
The idea is to catch cases where is a new row.

I know , there are many code enhancements to be done. I just posted an
initial code how this can work. Now we can enhance it. ie:

ValueJXPathBinding and UniqueFieldJXPathBinding are very similar a
enhancement can be using a base class for both.

Best Regards,

Antonio Gallardo