[Axis2] A few comments

[Glen Daniels <gd...@sonicsoftware.com>]

I haven't had time to do the serious review I'd like (this weeked was
taken up with being sick and working with dims on Axis 1.2 and I've been
caught up in some other stuff since), but here are a few initial
comments.  Reading more now, so more to come; better to have several
smaller messages than one big one anyway. :)  Talk to you guys in a
couple of hours!


* How about the data files for the tests residing in the same directory
as the test source (or better yet, under src/test/name/data), and that
way we could get them using getResourceAsStream()?  Much easier to know
where the relevant files are that way, not to mention the fact that you
wouldn't necessarily have to be connected to a particular directory to
run the tests.

* Shouldn't tests be in package "test.org..." or at least
"org.apache.foo.test"?  That removes the possibility of potential name
clashes between test classes and "real" classes, since they're typically
being compiled to the same classes directory.

* Shouldn't we have some sort of parsing infrastructure for the
config/deployment stuff?  There's an awful lot of repeated code
(checking for END_DOCUMENT events, etc) in there.  In fact, I'm not even
sure why we're bothering to use pull parsing for this stuff - why not
just use DOM (or maybe... OM? :))?  Especially now that we've got
directory-based deployment (and hence are pulling from multiple XML
documents), the chances that any individual file is going to be
prohibitively large are pretty slim.  Wouldn't OM/DOM be a lot easier?

* We discussed getting rid of org.apache.axis.impl and putting impls
either next to (org.apache.axis.ThingImpl) or below
(org.apache.axis.impl.ThingImpl) the interfaces.  I'd like to see this
refactor before release.

--Glen

Re: [Axis2] A few comments

[Srinath Perera <he...@gmail.com>]

Hi Glen;
> * How about the data files for the tests residing in the same directory
> as the test source (or better yet, under src/test/name/data), and that
> way we could get them using getResourceAsStream()?  Much easier to know
> where the relevant files are that way, not to mention the fact that you
> wouldn't necessarily have to be connected to a particular directory to
> run the tests.
+1 for use getResourceAsStream insted of the directly relay on the dir. 
Right now the test resources reside in the src/test-resources,  of the
being the usual maven convension. Only things is that it does not
point out which  test case have which resources.
Am Nutral on this to go on the either way

> * Shouldn't tests be in package "test.org..." or at least
> "org.apache.foo.test"?  That removes the possibility of potential name
> clashes between test classes and "real" classes, since they're typically
> being compiled to the same classes directory.
Here I am agree with the Ross (see the other answer)
 
> * Shouldn't we have some sort of parsing infrastructure for the
> config/deployment stuff?  There's an awful lot of repeated code
> (checking for END_DOCUMENT events, etc) in there.  
yap we could improve that code for sure;

> In fact, I'm not even
> sure why we're bothering to use pull parsing for this stuff - why not
> just use DOM (or maybe... OM? :))?  Especially now that we've got
> directory-based deployment (and hence are pulling from multiple XML
> documents), the chances that any individual file is going to be
> prohibitively large are pretty slim.  Wouldn't OM/DOM be a lot easier?
>I got few arguments for Stax :)
1) It does not make a another copy in the memeory, even though it will
not involve a big panelty it is plus point
2) Stax parsing is matter of getting use to the parser, having written
lot of stax parsing code it seems to much easier to me. I positive the
both apporches can be broken down to a algorithm of similer
complexity.
e.g.
 say one need to write a service.xml parsing code 

Stax code
========
int type = getType(xpp);
switch(type)
case Constants.PARAMETER_TYPE:
           processTransport(xpp);
            break;
case .........

public int getType(xpp){
     int event = xpp.next();
     if(event == Constants.START_ELEMNT){
           if("trasport".xpp.getLocalName()){
                 return Constants.PARAMETER_TYPE         
          }......
     }.....{
    }else if(event == Constants.END_DOCUMENT){
               error();          
   }

}


> * We discussed getting rid of org.apache.axis.impl and putting impls
> either next to (org.apache.axis.ThingImpl) or below
> (org.apache.axis.impl.ThingImpl) the interfaces.  I'd like to see this
> refactor before release.
sure, let finalize which one at the chat
Thanks
Srinath