[jboss-dev-forums] [JBoss AOP Development] - Re: ClassPool Refactoring

[flavia.rainone@jboss.com]

Kabir has provided me feedback on this in private. He thinks that, overall, the integration is good to be ported to trunk. I'll post here the points he addressed in our private chat. These are the points that need to reviewed:

1. The field ClassPool classPool of Instrumentor should be of type AOPClassPool.
2. I should see if I can find a workaround for checks such as the one below:
 if (this.childFirstLookup != delegate.childFirstLookup)
  |       {
  |          delegate.childFirstLookup = this.childFirstLookup;
  |       }
This check is there because childFirstLookup is a public field of ClassPool. AOPClassPool uses ClassPool as a delegate, so it can delegate to any classpool of jboss-classpool project. However, to force AOPClassPool and ClassPool to work consistently, I added these checks. This will be reviewed, but we are both afraid that maybe there is no solution for this.
3. Should ClassPoolRepository.callback be a list instead? I've made it a reference to a single callback for performance reasons. I can change it if Kabir or Ales prefer.
4. Maybe we should rename ClassLoaderRepository to avoid confusion with ClassPoolRepository class.
5.In (AOP)VFSClassLoaderDomain, instead of the initMapsDone and cleanupLoaderDone methods, I could just override initMaps() and cleanupLoader() directly in AOPVFSClassLoaderDomain, call the super method and add the "Done" stuff at the end. Kabir pointed out that this may be not so simple as it looks because I need some local variables from those methods.

I'm going to work on these points before porting the changes to trunk.

Notice that I'll have to check if the tests are still passing and do any adaptation needed on the 742 branch... This can be done only after the classpool project is estabilized and the vfs tests are passing ok. Those tests are the subject of another thread.
http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4265325#4265325

View the original post : http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4266376#4266376

Reply to the post : http://www.jboss.org/index.html?module=bb&op=posting&mode=reply&p=4266376