[jdom-interest] Important proposal: Element/Document changes

[Jools Enticknap]

<snip/>

<Element>

> First, on Element:
> 
> String getText()
>   Returns untrimmed text content.  Replaces getContent(true).  Content
> just isn't the right name for the text nodes.

+1 

> 
> String getTextTrim()
>   Returns trimmed text content.  Replaces getContent() and
> getContent(false).  Another possible naming is getTextTrimmed().
> getTextTrim() is currently the best candidate because it's shorter
> and matches foo.trim().

+1 for getTextTrim().


> 
> Element setText(String)
>   The natural replacement for setContent(String).

+1 

> 
> Element getChild(String name)
> Element getChild(String name, Namespace ns)
> List getChildren()
> List getChildren(String name)
> List getChildren(String name, Nmespace ns)
> -- or --
> Element getChildElement(String name)
> Element getChildElement(String name, Namespace ns)
> List getChildElements()
> List getChildElements(String name)
> List getChildElements(String name, Namespace ns)
>   These methods return (respectively) the first Element with the given
> name, all child elements with any name, and all Elements with the given
> name.  We'll remove getChild(String name, String uri) and
> getChildren(String name, String uri) because they prove a point but
> aren't very useful.  It's an open question whether the name should be
> getChild()/getChildren(), which are short and convenient, or
> getChildElement()/getChildElements(), which are more explicit and more
> in line with the XML specification terminology.  Let us know what you
> think.

+1 

I prefer the getChildElement(), and the getChildElements().

My first justification is that for non native english speakers Child and
Children are not intuative. Element and Elements is.

Secondly it's explicity stating it's job, rather than implying it.

> 
> List getMixedContent()
>   Returns the full content of an element, replacing
> getMixedContent(true).  Remove getMixedContent(boolean) because any time
> you're interested in the low-level mixed content you presumably want to
> see all the whitespace nodes, or could at least ignore them yourself.
> Is that true?  Naming this method getContent() might be nicer but that
> will cause subtle bugs to existing code.  This also makes it clear the
> List may contain various types of objects.

+1 

This call should result in the list containing children with whitespace.

> 
> String getAttributeValue(String name)
> String getAttributeValue(String name, Namespace ns)
>   Returns the given attribute value, avoiding the getAttribute() call.
> I would have sworn we had this method already, but can't find record of
> it.  It's surely convenient.

+1

But what about;

List getAttributeValues(String name)
List getAttributeValues(String name, Namespace ns) 

void addAttributeValue(String name, String value)
void addAttributeValue(String name, String value, Namespace ns)

Should'nt the above also implemented for completeness ? Or am I missing
something obvious ?

> 
> Element addContent(String text)
> Element addContent(Element element)
> Element addContent(ProcessingInstruction pi)
> Element addContent(Entity entity)
> Element addContent(Comment comment)
>   Adds the given content to the element.  Replaces the various
> addChild() methods because we're not treating Comments and such as
> "children".  (Before you could call addChild(pi) and then call
> getChildren() and wouldn't find the method returning the pi!)

+0 

However getContent() will only return the textual content, and 
getMixedContent() will have to be called to get all the content added
using the addContent() method.

Mmmm, I'm not totally sure I like this.

> 
> boolean removeContent(String text)
> boolean removeContent(Element element)
> boolean removeContent(ProcessingInstruction pi)
> boolean removeContent(Entity entity)
> boolean removeContent(Comment comment)
>   Removes the given item.  Does a == check to compare, so presumably the
> item would have been just retrieved with a getMixedContent().  Replaces
> the current removeChild() methods that were accepting non-elements.

+1 for the names.

-1 for the == check. I would prefer a equals(), given that the boolean
   result will inform you of the result of operation.

> 
> boolean removeChild(String name)
> boolean removeChild(String name, Namespace ns)
> boolean removeChildren(String name)
> boolean removeChildren(String name, Namespace ns)
> -- or --
> boolean removeChildElement(String name)
> boolean removeChildElement(String name, Namespace ns)
> boolean removeChildElements(String name)
> boolean removeChildElements(String name, Namespace ns)
>   Removes the given child or children elements.  These methods already
> exist.  We'll remove the methods removeChild(String name, String uri)
> and removeChildren(String name, String uri).  Again, let us know which
> naming style you prefer and why.

+1 for removeChildElement(), removeChildElements().


</Element>


<Document>
> 
> Now some methods on Document:
> 
> Document addContent(Element root)
> Document addContent(ProcessingInstruction pi)
> Document addContent(Comment comment)
> Document addContent(Entity entity)   // Do we need this?
>   Similar to the methods on Element.  Cleans things up since there's no
> reason to have addComment(Comment) on Document but addContent(Comment)
> on Element.  The addContent() method will be checked so only one Element
> can be added to a document.

+1 for all except addContent(Element root).
   I'm guessing that addContent(Element root) will replace the previous
   root element ? then it's not adding its replacing.


> 
> boolean removeContent(Element element)
> boolean removeContent(ProcessingInstruction pi)
> boolean removeContent(Comment comment)
> boolean removeContent(Entity entity)  // XXX Do we need this?
>   Removes the given item.  Does a == check to compare, so presumably the
> item would have been just retrieved with a getMixedContent().  Replaces
> the current removeProcessingInstruction() method, and the others are new
> functionality.

+1 except I'm not in favour of the == compare. I'd prefer the .equals() 
   approach.

</Document>

--Jools