struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Apache Wiki <>
Subject [Struts Wiki] Update of "RoughSpots" by Bob Lee
Date Tue, 18 Apr 2006 23:08:56 GMT
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:

-   * 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
+   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:
For additional commands, e-mail:

View raw message