You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@abdera.apache.org by jm...@apache.org on 2006/08/21 22:20:36 UTC
svn commit: r433345 - in
/incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax:
FOMElement.java FOMFeed.java
Author: jmsnell
Date: Mon Aug 21 13:20:36 2006
New Revision: 433345
URL: http://svn.apache.org/viewvc?rev=433345&view=rev
Log:
If an element already contains a child, don't add it again.
Modified:
incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMElement.java
incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMFeed.java
Modified: incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMElement.java
URL: http://svn.apache.org/viewvc/incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMElement.java?rev=433345&r1=433344&r2=433345&view=diff
==============================================================================
--- incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMElement.java (original)
+++ incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMElement.java Mon Aug 21 13:20:36 2006
@@ -579,4 +579,9 @@
}
return locale;
}
+
+ public void addChild(OMNode node) {
+ if (node.getParent() == this) return;
+ super.addChild(node);
+ }
}
Modified: incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMFeed.java
URL: http://svn.apache.org/viewvc/incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMFeed.java?rev=433345&r1=433344&r2=433345&view=diff
==============================================================================
--- incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMFeed.java (original)
+++ incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax/FOMFeed.java Mon Aug 21 13:20:36 2006
@@ -133,6 +133,7 @@
@Override
public void addChild(OMNode node) {
+ if (node.getParent() == this) return;
if (isComplete() && node instanceof OMElement && !(node instanceof Entry)) {
OMElement el = this.getFirstChildWithName(ENTRY);
if (el != null) {
Re: svn commit: r433345 - in /incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax:
FOMElement.java FOMFeed.java
Posted by James M Snell <ja...@gmail.com>.
Yes, I'm going through the tests.
- James
Garrett Rooney wrote:
> On 8/21/06, jmsnell@apache.org <jm...@apache.org> wrote:
>> Author: jmsnell
>> Date: Mon Aug 21 13:20:36 2006
>> New Revision: 433345
>>
>> URL: http://svn.apache.org/viewvc?rev=433345&view=rev
>> Log:
>> If an element already contains a child, don't add it again.
>
> Uhh, ok, so something's definately wrong with this. This change
> results in about half the parser tests failing.
>
> -garrett
>
Re: svn commit: r433345 - in /incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax: FOMElement.java FOMFeed.java
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 8/21/06, jmsnell@apache.org <jm...@apache.org> wrote:
> Author: jmsnell
> Date: Mon Aug 21 13:20:36 2006
> New Revision: 433345
>
> URL: http://svn.apache.org/viewvc?rev=433345&view=rev
> Log:
> If an element already contains a child, don't add it again.
Uhh, ok, so something's definately wrong with this. This change
results in about half the parser tests failing.
-garrett
Re: svn commit: r433345 - in /incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax: FOMElement.java FOMFeed.java
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 8/21/06, James M Snell <ja...@gmail.com> wrote:
> That's likely a better approach. A challenge is that our API doesn't
> make it very obvious. For instance, I'm helping to debug some internal
> code that ended up doing the following:
>
> Link link = Factory.INSTANCE.newLink(feed);
> ...
> feed.addLink(link);
>
> The newLink adds the link to the feed when the link is created. The
> addLink(link) causes it to be added again.
Perhaps an assert instead of the if, so people can at least find the
problem when they make that mistake? I'm not sure how this sort of
thing is usually handled in Java...
FWIW, it probably doesn't help that Factor.newLink(Element) doesn't
actually have any javadoc, so there's no way for anyone to know that
it adds the link to the element unless they've read the source.
-garrett
Re: svn commit: r433345 - in /incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax:
FOMElement.java FOMFeed.java
Posted by James M Snell <ja...@gmail.com>.
That's likely a better approach. A challenge is that our API doesn't
make it very obvious. For instance, I'm helping to debug some internal
code that ended up doing the following:
Link link = Factory.INSTANCE.newLink(feed);
...
feed.addLink(link);
The newLink adds the link to the feed when the link is created. The
addLink(link) causes it to be added again.
- James
Garrett Rooney wrote:
> On 8/21/06, jmsnell@apache.org <jm...@apache.org> wrote:
>> Author: jmsnell
>> Date: Mon Aug 21 13:20:36 2006
>> New Revision: 433345
>>
>> URL: http://svn.apache.org/viewvc?rev=433345&view=rev
>> Log:
>> If an element already contains a child, don't add it again.
>
> Why would there be any code doing this? It seems like the kind of
> case that's only an error, should we really let it happen and slow
> everything down with the extra if, or should we just fix the code that
> tries to do it?
>
> -garrett
>
Re: svn commit: r433345 - in /incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax: FOMElement.java FOMFeed.java
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 8/21/06, jmsnell@apache.org <jm...@apache.org> wrote:
> Author: jmsnell
> Date: Mon Aug 21 13:20:36 2006
> New Revision: 433345
>
> URL: http://svn.apache.org/viewvc?rev=433345&view=rev
> Log:
> If an element already contains a child, don't add it again.
Why would there be any code doing this? It seems like the kind of
case that's only an error, should we really let it happen and slow
everything down with the extra if, or should we just fix the code that
tries to do it?
-garrett