* This method in ConfigurationManager is wrong:
Interceptor getInterceptor(String clazz)
It assumes that there is only one instance of each interceptor class. This does not account for the case where one instance is used with many names (compare with servlets), and configured differently in each case. It has to change to:
Interceptor getInterceptor(String namespace, String name)
Patrick and I have talked about parameterized Interceptors but we were unable to come up with an example yet, so we didn't implement it (YAGNI in action). I agree that if we decide to have parameterized Interceptors, this is necessary.
Ok, well I guess this argument is the same as parameterized commands. I have a couple of interceptors in my own AOP application that are indeed parameterized, so at least in that context there's a need, and I can make a similar example for this case (this, interestingly enough, touches upon the parameterization issues of commands too that I mentioned in my last post).
* the getAll* methods in the PackageContext's should not be there. The contexts may understand their own stuff, and that they happen to have a parent, but they should not be responsible for "putting it all together". That's the task of the ConfigurationManager IMHO. I'm not even sure a PC should have a direct link to it's parent. Maybe the name is enough, and the CM should make the connection.
Well, maybe. This just made a lot of things easier. I'll have to look in the code again for more specific examples.
Just move them to the CM.
Some issues not related to configuration:
* It seems ActionInvocation's are called directly by code. A better way to do this is to create an ActionProxy which upon execute() creates an invocation and invoke it. This ensure that invocations are not reused. In general an "invocation" is not a good thing to deal with directly since it's conceptually a side-effect of something else. Hiding it behind an ActionProxy would fix this.
Sounds like a good idea, but how do we prevent people from using ActionInvocations directly? Would we make them package local and put the ActionProxy in there as well?
I'm not sure why people would try to use it directly, but using package protected constructors would work.
In what way?
Well, mainly the good 'ol curly braces thing (and I have thus far never seen a good reason why one would put 'em on the same line), and also how portions of the class is delineated. I prefer the template used in WW, which had more clearly defined sections.
/Rickard
------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf _______________________________________________ Opensymphony-webwork mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/opensymphony-webwork
