axis-java-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eran Chinthaka" <chinth...@opensource.lk>
Subject RE: [Axis2] Some comments on SAAJ toy
Date Thu, 31 Mar 2005 05:33:44 GMT
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:jayachandra@gmail.com]
> Sent: Thursday, March 31, 2005 10:45 AM
> To: axis-dev@ws.apache.org; chinthaka@opensource.lk
> 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
> <chinthaka@opensource.lk> 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




Mime
View raw message