Re: [dom4j-dev] Why do add(...) methods return void?

[James Strachan]

Hi Thomas From: Thomas Nichols This sounds good - though see my comments to James E. re limitations of parseText(). . BTW, why is it "setAttribute" rather than "addAttribute" ?   Actually right now its setAttributeValue(name,value) to set the value and attributeValue(name) to retrieve the value. The main reason for the difference is that an element can only have one value for a given name so calling setXX seemed more appropriate than addXX. Though maybe I'm having second thoughts.   Would people prefer addAttribute?   Element foo = doc.addElement( "foo" ).addAttribute( "id", "123").addText( "some text" );   It is more consistent I suppose. I'm kinda tempted now...   It may be quite confusing for users, but we could let the add() method return a reference to the Element on which the node was added since its not acting as a factory method as you pass in the node you wish to add. This sounds a workable approach - but would it mean copy-and-paste of code currently in AbstractBranch.java into both AbstractElement.java and AbstractDocument.java? Your call, I think, as to whether the (possibly) improved accessibility is worth duplicating code.   Moving these few methods into AbstractElement and AbstractDocument would be fine with me (its really not much code and its more 'plumbing' code rather than 'real' code anyway). Maybe some package-visibility methods in DocumentHelper that abstract out the commonality, the AbstractElement and AbstractDocument impls just call the helpers and do a bit of casting? Sometimes, on these occasions, C++ takes on a faintly rosy hue... until you remember the memory leaks and core dumps.   Agreed. I miss being able to do mixins in Java. Though like C++ templates, pointers and operator overloading, its only on rare occasions I miss them. Yes, this is elegant... Element html     = factory.createElement( "html" )         .add(             factory.createElement( "head" )                 .add(                     factory.createElement( "title" )                         .addText( "this is the title" )                 )         )         .add(             factory.createElement( "body" )                 .add(                     factory.createElement( "h1" )                         .addText( "title #1" )                 )                 .add(                     factory.createElement( "p" )                         .addText( "this is paragraph #1" )                 )                 .add(                     factory.createElement( "p" )                         .addText( "this is paragraph #2 " )                         .add(                             factory.createElement( "b" )                                 .addText( "bold" )                         )                         .addText( " more text" )                 )         );       Agreed. After writing the above code, I kinda like it now. Its a shame we can't think a way of writing the above using addElement() rather than the add() method to avoid the factory.createElement() lines, it would be much simpler then... Absolutely. I'm seldom just printing static text, though, it's usually intermingled static and dynamic content. (And for the above you should almost certainly be using XMLC anyway :-)     I've never quite got into XMLC. I wonder if we need a dom4j equivalent? 1. Agreed, Element.addElement() returns the child.     Cool. 2. Declaring add() to return anything at all is not what I'd expect, by analogy to the Collections family, but that's no big deal.     Agreed. I'm not totally sold on the add() approach but I think it makes sense. I'll sleep on it some more.     3. If add() is to return the added Branch, I'd much prefer add (Element) to return Element, not Branch - cast ye away the works of Mammon.       Agreed - it must return an Element or Document. We can just leave the add(Node) to be the polymorphic method.     4. I'm confused about the distinction between addXYZ and setXYZ methods - I don't expect setXYZ(...) to create a new object.     Agreed.   What are others thoughts on renaming setAttributeValue() -> addAttribute()?   James