Chamikara,
When is the planned date for the merge? I have some code to add, but will
wait till u finish the merge.
Also I believe that Deepal will help us with the Listerner stuff :)
Regards,
Rajith
On 2/22/07, Chamikara Jayalath <[EMAIL PROTECTED]> wrote:
Hi Glen,
On 2/22/07, Glen Daniels <[EMAIL PROTECTED]> wrote:
>
> Hey folks:
>
> David Illsley wrote:
> > Here's a diif of the branch for those who are interested.
>
> Thanks for the summary David!
>
> I took a quick look at the diff and have a few comments for the folks
> working on this stuff. I haven't yet fully grokked the implications of
> all this, so there may be a few more architectural comments forthcoming
> after a more detailed read (and after I finish tuning my head all the
> way back in to Axis2!).
>
> * Can we please remove all message strings from the code itself and
> resource-ize them?
* Noticed a few code cleanliness issues like overly long lines and
> missing space around operators... maybe run this through someone's
> IDE code-cleanup?
I am doing this. Will make sure this is done before the commit.
* Rather than continually repeating
>
> context.setClustered(true);
> clusterManager.addContext(context);
>
> why not just have clusterManager.addContext() be responsible for
> setting the clustered flag on the context? Cleaner code, and
> you won't have potentially incorrectly set clustered flags in
> case of errors.
OK.
* Take a look at this code in AbstractContext:
>
> > + public void flush () throws AxisFault {
> > + //if clustering is enabled, ClusterManager will be called to
> replicate the context state.
> > + if (clustered) {
> > +
> > + ClusterManager clusterManager = null;
> > +
> > + if (this instanceof ConfigurationContext) {
> > + ConfigurationContext configurationContext =
> (ConfigurationContext) this;
> > + clusterManager =
> configurationContext.getAxisConfiguration().getClusterManager();
> > +
> > + } else if (this instanceof ServiceGroupContext) {
> > + ConfigurationContext configurationContext =
> (ConfigurationContext) this.getParent();
> > + if (configurationContext==null) {
> > + String message = "The parent of the
> ServiceGroupContext has not been set";
> > + throw new AxisFault (message);
> > + }
> > +
> > + clusterManager =
> configurationContext.getAxisConfiguration().getClusterManager();
> > +
> > + } else if (this instanceof ServiceContext) {
> > + ServiceGroupContext serviceGroupContext =
> (ServiceGroupContext) this.getParent();
> > + if (serviceGroupContext==null) {
> > + String message = "The parent of
> the ServiceContext has not been set";
> > + throw new AxisFault (message);
> > + }
> > +
> > + ConfigurationContext configurationContext =
> (ConfigurationContext) serviceGroupContext.getParent ();
> > + if (serviceGroupContext==null) {
> > + String message = "The parent of
> the ServiceGroupContext has not been set";
> > + throw new AxisFault (message);
> > + }
> > +
> > + clusterManager =
> configurationContext.getAxisConfiguration().getClusterManager();
> > + }
>
> This kind of "if (this instanceof SubClass)" structure is usually
> a signal that there's some more elegant way to get the same job done.
> In this case, I'd suggest either a) just put the ClusterManager
> member in the AbstractContext itself, or (even better) introduce
> a "Clusterable" interface which contains the getClusterManager()
> method. Either of these solutions allows extensibility and makes
> the above code much cleaner.
>
> More later...
The reason for this was not having a abstract method to get the
ConfigurationContext. I added a getRootContext() abstract method to the
AbstractContext which will do the trick.
Thanks a lot for the info.
Chamikara
Thanks,
> --Glen
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
>
