RE: [Axis2] Some comments on SAAJ toy

[Shahi, Ashutosh]

See comments Inline From: Eran Chinthaka [mailto:[EMAIL PROTECTED] Sent: Thursday, March 31, 2005 8:20 AM To: axis-dev@ws.apache.org Subject: [Axis2] Some comments on SAAJ toy     Hi Jaya, Ashu and Venkat,   I went through the code that you have put in here are some of my comments.   its better not to use new OMElementImpl() sort of things within the code. We have introduced OMFactory to create objects, simply to smooth switching of OM implementations. Mind you that OMElementImpl() is specific to linked list model only. So its better if you can use omFactory.createOMElement() methods. [Ashu] We were thinking along the lines of using omFactory with the MessageFactory in SOAP. Since we haven’t implemented MessageFactory yet, we hardcoded impl classes. But I think it’s a good suggestion and I’ll make these changes   SOAPElementImpl.java  addChildElement(Name) method. I think code should be changed a bit.   org.apache.axis.om.OMElement newOMElement = new org.apache.axis.om.impl.llom.OMElementImpl(new QName(name.getURI(), name.getLocalName(), name.getPrefix()), omElement); omElement.addChild(newOMElement); return (SOAPElement)(new SOAPElementImpl(newOMElement));   Better be changed to   org.apache.axis.om.OMElement newOMElement = OMFactory.newInstance().createOMElement(new QName(name.getURI(), name.getLocalName(), name.getPrefix()), omElement); omElement.addChild(newOMElement); return new SOAPElementImpl(newOMElement);   [Ashu] Will do; this is an example of 1 I guess   All the addChildElement methods have explicit type cast to SOAPElement from SOAPElementImpl, which IMHO is not required. removeContents has a comment with a question mark. Well what you can do is get the children of the current node and call the detach method of all of them. [Ashu] We had left empty implementation for many methods in the SOAPElement class on which we had questions (as it was too DOM specific), removeContents included; and thought will come back to them once we get feedback and suggestions on how to implement. the getVisibleNamespacePrefixes() currently returns declared namespaces only. But according to the javadoc found on the SAAJ interfaces, this should return all the “visible” namespaces. So better get the namespace of the parents as well. [Ashu]  Agreed. Apart from this I have a question. The OMNamespace used in OM doesn’t have a concept of LocalName. Also omElement allows to declare the namespace (more than 1) in the context of current element and stores them in a hashmap, but I could not see a way to find out which one was used to create the current node and which others are just declared in current context. Because of this could not implement methods like getNamespaceURI(), getPrefix() in NodeImpl class. Any thoughts on this?   Sorry I didn’t have enough time go through all the code, due to the summit. I went through only the SOAPELement class. Let me have some more time to go through this again.   Anyway, can I also write some code in to your effort ?   Sure, you are most welcome to help us with code :). I’ll also make some of the changes suggested by you and post them.    I wonder whether I can do that as its under the name jaya_ashu_vankat scratch ;) ;)     Regards, Eran Chinthaka