Hi Devs,

AxisServlet has undergone many changes and apparently there is room for code
improvements.  Looking at the code  it is  evident that some code has
repeated many times and some helper methods has only slight differences,
that could have been coupled into fewer helper methods. In fact some of the
helper methods in AxisServlet should have been in a util such that other
Listener developer folks could easily use with.

Following are the improvements that should have been done IMO.

1. protected MessageContext
createAndSetInitialParamsToMsgCtxt(HttpServletResponse resp,
                                      HttpServletRequest req) throws
AxisFault { ...}

  and

  protected MessageContext createMessageContext(HttpServletRequest req,
                                                 HttpServletResponse resp)
throws IOException {..}

apparently above methods will do the same thing; but prior use in POST
request and later use in GET request. Why? one method would be more than
enough for the most cases.

2. For a RESTFul invocation; it uses RESTUtil class. But to process a SOAP
request it uses inline code [line 268 - 341] [doPost()]. There also exist an
SOAPUtil (org.apache.axis2.transport.http.util.SOAPUtil) for processing SOAP
request. But no one uses it. The worst scenario is; SOAPUtil has code for
processing SOAP request but no one cares of updating the code when ever they
change the code in doPost(). IMHO this is really bad and either we should
move the logic to SOAPUtil and use it or just remove the obsolete class.
But IMHO we should move the code to SOAPUtil and use that class instead. If
some listener developer wants it; one could easily use it.

3. It is apparent that most of the parts of the doPost(); doGet(); doPut()
and doDelete() are some. Most of the time code repeat itself. This is not
good either in  code "close for modification" or "open for extension".
Should have moved to a util methods and reuse it.

4. Same goes to handleFault methods as well.

5. What is the purpose of inner class ServletRequestResponseTransport. Does
this handle all the media types?

Should I've raised a JIRA on this. Before that please express you consensus
on the prior.

Thank you

Saminda

--
Saminda Abeyruwan

Software Engineer
WSO2 Inc. - www.wso2.org

Reply via email to