Mailing-List: contact axis-dev-help@ws.apache.org; run by ezmlm
Precedence: bulk
Reply-To: axis-dev@ws.apache.org
Received-SPF: neutral (herse.apache.org: local policy)
Subject: Re: [Axis2] Issues with the current code base
From: Sanjiva Weerawarana <sanjiva@opensource.lk>
To: axis-dev@ws.apache.org
In-Reply-To: <1170084449.10298.68.camel@scylla.watson.ibm.com>
References: <45BD9023.4050008@opensource.lk>
	 <1170084449.10298.68.camel@scylla.watson.ibm.com>
Content-Type: text/plain
Organization: Lanka Software Foundation
Date: Mon, 29 Jan 2007 23:42:05 +0530
Message-Id: <1170094326.3447.29.camel@localhost.localdomain>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit

Bill, IMO I made a mistake in lifting my -1 on this patch .. this
could've and should've gone in as an auxiliary bit of code without
modifying MC at all. Calling mc.activateMessageContext on *every*
message that comes in seems like a major ovehead to the mind even if not
to the body (you know what I mean). There was no technical need
whatsoever for this code to be in MC itself for Matt to be able to do
whatever he needs to make Sandesha persist using Java object
serialization instead of the serialization architecture that exists
now. 

Next time I won't give in so easy :).

In any case, I'm yet to see an explanation from Anne for the algorithm
used to figure out the parent service context etc. for a message
context. Anne, can you explain how you're going about figuring those
refs out please? Please don't say RTFS :(.

We need to performance compare 1.1.1 with the current head and see what
the impact of this change is. 

On code conventions- search the archives please .. we've had lots of
discussion on this in the early days and decided on the conventions. I'm
pretty sure we put them in the wiki somewhere but have no idea where off
the top of my head :(. Interestingly, even the original JAX-WS proposal
by IBM mentions coding conventions:
http://wiki.apache.org/ws/FrontPage/Axis2/JAX-WS-Proposal. so maybe
someone in IBM found a ref?

Sanjiva.

On Mon, 2007-01-29 at 07:27 -0800, Bill Nagy wrote:
> Hi Deepal,
> 
> On Mon, 2007-01-29 at 11:41 +0530, Deepal Jayasinghe wrote:
> > Hi all;
> > 
> > I just went though the current code base and realized that
> > MessageContext code has been changed a lot . I found few issues with the
> > code base and hope we need to fix them. So I thought of sending this
> > mail for everyone's consideration.
> > 
> > Well someone please explain to me whose going to need MessageContext
> > serialization ,
> >  Chamikara : Do you want that for Sandesha ?
> >  Ruchith : Do you want that for Security ?
> > If none of you want this , who else need this ?
> 
> As Matt pointed out, we went through this on an earlier discussion --
> Sandesha is the intended consumer.
> 
> 
> > I am asking this question b'coz introduction of MC serialization we have
> > the following for each req.
> >  - When ever Axis2 received a message it calls 
> > activateMessageContext(msgContext);
> >  - And what that does is , it calls mc.activate(engineContext); method
> > 
> > Unfortunately that method is TOO long and was very difficult to
> > understand:). Ann can you simplify the method (that will be very helpful) .
> 
> Actually, if you notice, the first line of MessageContext.activate(...)
> is a short-circuit test on needsToBeReconciled, which is only ever set
> when the MessageContext (and related parts) are deserialized using the
> MessageContext deserialization routines -- for all other cases, it ends
> up being a no-op so you can stop reading at that point 8-].  As for the
> method being too long, of the 306 lines in that method, 110 are
> comments/log messages, and the rest of the code consists of
> if-not-null-invoke-a-method blocks.  Unfortunately the MessageContext
> has a lot of handles to other objects, so there need to be a number of
> those tests.  If you're having difficulty understanding a particular
> section of that method, please ask and we'd be happy to explain it to
> you.  
> 
> I certainly think that the rest of the code could use some "code
> bloat" (i.e. comments) -- e.g. there are no class-level comments on
> ListenerManager, AxisConfiguration, PhaseResolver (just to name a few.)
> 
> >  
> > In the meantime code convention in MC has changed a lot and need to have
> > very consistent code convention, please do not differ form that.
> 
> This is the second time that "code conventions" have been mentioned
> (Sanjiva brought this up in an earlier discussion as well) -- I was not
> aware of these, and was unable to find anything in the docs that talk
> about them.  (The Developer Guidelines only talk about the mailing
> list.)  Could you please point me to where these are codified?
> 
> > Among the all , the most worst thing I saw in the code is following kind
> > of things, I strongly believe we should not have this kind of code in
> > the code base, If you found such kind of code please point out them then
> > and there.
> > 
> >  - String tmpClassNameStr = "null";   (is this the way we initialize to
> > NULL )
> >  - String tmpHasList      = "no list"
> >  - Unnecessary casting
> >  - A number of unused variables
> >  - Variable declarations here and there  (as an example private static
> > final String  - selfManagedDataDelimiter = "*";)
> 
> I'm indifferent on the first two; in some cases it makes the code easier
> to read and debug at the cost of an assignment and space in the string
> table.  The third one should be caught by any decent compiler and
> eliminated (so long as you're not casting back and forth) and again
> sometimes enhances readability so I'm indifferent on this one as well.
> I agree on the fourth -- I don't think that there's ever a good case for
> extraneous variables.  The fifth is again a code readability issue and
> one of the reasons that Java doesn't require that you declare everything
> up front. 
> 
> -Bill
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
> For additional commands, e-mail: axis-dev-help@ws.apache.org
> 
-- 
Sanjiva Weerawarana, Ph.D.
Founder & Director; Lanka Software Foundation; http://www.opensource.lk/
Founder, Chairman & CEO; WSO2, Inc.; http://www.wso2.com/
Director; Open Source Initiative; http://www.opensource.org/
Member; Apache Software Foundation; http://www.apache.org/
Visiting Lecturer; University of Moratuwa; http://www.cse.mrt.ac.lk/


---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org