You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@abdera.apache.org by Garrett Rooney <ro...@electricjellyfish.net> on 2006/08/21 22:24:26 UTC
Re: svn commit: r433345 - in /incubator/abdera/java/trunk/parser/src/main/java/org/apache/abdera/parser/stax: FOMElement.java FOMFeed.java
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, 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
>