Jaya, got you. That's the same reason why I wanted to help you. (Anyway Venkat is next to me, and he wanted me to comment ;) )
Some more comments .. Global Comments : . I saw in lot of places, not only in SOAPElementImpl, that you guys have used new OMElementImpl(). Shall we all stick to OMFactory methods to create new OM objects. This is sort of a good practice as its giving flexibility to change OM impls smoothly. . There were some explicit cast which is not required to do. For example DetailEntryImpl implements DetailsEntry and there were casting to DetailEntry from DetailElementEntry when returning in methods. Anyway, Can I help you by implementing some stuff in your scratch area, if time permits ? -- Chinthaka > -----Original Message----- > From: jayachandra [mailto:[EMAIL PROTECTED] > Sent: Thursday, March 31, 2005 10:45 AM > To: [email protected]; [EMAIL PROTECTED] > Subject: Re: [Axis2] Some comments on SAAJ toy > > Thanks Eran for the inputs. Actually, this effort was more in the > direction of creating a proof of concept trail. So, many methods were > deliberately left uncoded or partially coded. So far, what is present > is some minimal code that's required for the test case to pass and > thereby convince ppl of this idea of SAAJ wrapping. Nevertheless, the > (SOAPElement) typecasting and other such things can be cleaned up. And > if the team gets a clean chit to go ahead and make this a > comprehensive enough implementation we can all work together and make > it as solid as possible :) > > Jaya > > On Thu, 31 Mar 2005 08:50:15 +0600, Eran Chinthaka > <[EMAIL PROTECTED]> wrote: > > > > > > > > > > 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. > > 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); > > > > > > 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. > > 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. > > > > > > > > > > > > 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 ? > > > > > > > > I wonder whether I can do that as its under the name jaya_ashu_vankat > > scratch ;) ;) > > > > > > > > > > > > Regards, > > > > Eran Chinthaka > > > -- > -- Jaya
