Dear Wiki user, You have subscribed to a wiki page or wiki category on "Struts Wiki" for change notification.
The following page has been changed by Bob Lee: http://wiki.apache.org/struts/RoughSpots ------------------------------------------------------------------------------ - * Looking up a `ResultConfig` should be a one-liner. Right now you have to: {{{ + 1. Looking up a `ResultConfig` should be a one-liner. Right now you have to: {{{ - ActionConfig config = invocation.getProxy().getConfig(); + ActionConfig config = invocation.getProxy().getConfig(); - Map results = config.getResults(); + Map results = config.getResults(); - ResultConfig resultConfig; + ResultConfig resultConfig; - synchronized (config) { + synchronized (config) { - resultConfig = (ResultConfig) results.get(resultCode); + resultConfig = (ResultConfig) results.get(resultCode); - } + } }}} + 1. We don't really need the `Action` interface anymore. Should we get rid of it? It has constant fields for result names. Should we move these to a class named `ResultNames` and encourage users to static import them as needed? + + 1. Only put classes in root package that most users need to know about. For example, most don't need to know about `Default*` or `ObjectFactory`. + + 1. Only make classes/members public if we're willing to guarantee future compatibility. Everything else should be package-private or excluded from the Javadocs. + + 1. Remove `destroy()` and `init()` from `Interceptor`. They don't make much sense until the interceptor lifecycle is specified (see next item). I've never needed them, yet it's a pain to implement empty methods every time I implement an interceptor. Users can use the constructor/finalizer or we can create additional lifecycle interfaces. + + 1. Specify `Interceptor` lifecycle. Right now if you apply an interceptor to a single action, you get a new instance every time. If you define an interceptor in a stack, the same instance gets reused. + + 1. Get rid of `AroundInterceptor`. Having `before()` and `after()` methods doesn't make things simpler. It reduces flexibility. You can't return a different result. You can't handle exceptions cleanly. The actual interceptor class doesn't appear in the stack trace (you see `AroundInterceptor` over and over). + + 1. Try to get rid of thread locals: `ActionContext` and `ServletActionContext`. At least make them package-private. Sometimes interceptors need access to the servlet API. In this case, they should implement a servlet-aware interceptor interface. For example: {{{ + class MyInterceptor implements HttpInterceptor { + public String intercept(HttpInvocation i) { + HttpServletRequest request = i.getRequest(); + ... + } + } + }}} + + 1. Is `ValidationAware` a good name? Perhaps `Errors` or `ErrorList` would be a better name. + + 1. Merge `ActionContext` and `ActionProxy` into `ActionInvocation` (at least from the users' perspective). Better specify what happens during chaining/action tags. + + 1. I'm not sure I'd expect `ActionInvocation.getResult()` to recurse over chain results. Maybe we should have two methods? `getResult()` and `getFinalResult()`. I've never needed this so I don't know. + + 1. `ActionInvocation.invokeActionOnly()`. Does this need to be public? Sounds dangerous. + + 1. Eliminate non-private fields. Protected fields in `ActionConfig` for example. + + 1. Rename `ActionInvocation` to `Invocation` or `Request`. Shorter is better. + + 1. `TextProvider` is a bad name. The JDK refers to these as "messages" everywhere. + + 1. Come up with a clean way to separate "view" actions from "update" actions. For example, you may have `view()` and `update()` methods in the same action class. Right now XWork has `MethodFilterInterceptor`, but that's not a very clean solution. You don't want validation or the `DefaultWorkflowInterceptor` to run for the `view()` method. My current project has separate interceptor stacks, but it would be nice if there was some first class support for this. We could flag action invocations as "view" or "update" (using an enum). We could automatically choose a mode based on whether the request is an HTTP GET or POST. Or we could set the mode based on an annotation on the action method. Or some other way... + + 1. On the OGNL value stack `#request` refers to request attributes and `#parameters` refers to the parameters. I think we should rename these `#request` for request parameters and `#requestAttributes` for request attributes. + + == Nice to haves == + + 1. Inheritance is a sucky way to reuse code between actions. It would be nice if the framework had built-in support for mixins using cglib or Spring. For example, instead of me extending a class that implements `ValidationAware`, Struts can extends my action class at runtime and implement the `ValidationAware` methods by delegating them to another object (a mixin): {{{ + abstract class MyAction implements Validateable, ValidationAware { + + public void validate() { + addActionError(...); + } + } + }}} + --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
