[Struts Wiki] Update of "RoughSpots" by DonBrown

[Apache Wiki <wi...@apache.org>]

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 DonBrown:
http://wiki.apache.org/struts/RoughSpots

------------------------------------------------------------------------------
      * [jcarreira] I'm not sure about this... The Action interface is kind of just a marker interface, but at least it gives us SOMETHING to point users to
      * [crazybob] I'll buy that. We do need to move the constants out and encourage users to use static import (Effective Java Item 17).
      * [plightbo] Related to this, I would encourage us to try to find a solution (using Bob's mix-in suggestion below, or possibly just taking advantage of the value stack) that would make ActionSupport much simpler. This would encourage using POJOs more.
+     * [mrdon] Regardless whether we remove Action or not, I like the idea of moving the result constants out.  
  
    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`.
      * [plightbo] +1 on this - sounds like Bob has a good handle on what it takes to make a nice API. I'll defer to him on this.
+     * [mrdon] +1
  
    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.
    
      * [plightbo] + 1 again.
+     * [mrdon] This I don't agree with.  From a framework developer, I understand the logic, but from a user, it is arrogant.  I think we should allow people to extend Struts in ways we haven't imagined, and restricting access like this says, "We know more than you and will force you to do it our way."
  
    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.
  
@@ -45, +48 @@

      * [jcarreira] The idea was that people would forget to do invocation.invoke() and be confused... Easier for users just to implement a before() method when that's all they need. I agree on the stack traces though.
      * [crazybob] It's kind of hard to forget to call `invocation.invoke()`; you have to return something. ;) Interceptors are already an "expert" feature anyway.
      * [plightbo] Big +1.
+     * [mrdon] +1 as well
  
    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 {
@@ -58, +62 @@

      * [jcarreira] These 2 are orthogonal... Getting rid of ThreadLocals is problematic. I think we'd end up breaking 90% of old WebWork apps if we did, and it's still not clear that everything could be covered if we did... I like the idea though, and Patrick and I really wanted to do this out of the gate, but backwards compatibility with WebWork 1.x at a macro-level made us think otherwise...
      * [crazybob] Interceptors need access to the servlet API. They shouldn't have to call a `ThreadLocal` if we can avoid it and they shouldn't need to cast. We shouldn't worry about breaking old WebWork apps (see new opening paragraphs). Let's get it right the first time around because we will not be able to fix it later. 
      * [plightbo] By running our interceptors and other objects through the same factories and lifecycle managers that the action classes go through, this should be a non issue.
+     * [mrdon] This I'd like to see.  I've found myself using these objects so often in wierd little places, I'd be loath to remove them unless we could prove 100% that their information can be retrieved elsewhere.
  
    1. Is `ValidationAware` a good name? Perhaps `Errors` or `ErrorList` would be a better name.
  
@@ -83, +88 @@

  
      * [jcarreira] We don't want to allow for extension?
      * [crazybob] Extension through interfaces and methods? Yes. Public/protected fields? Absolutely not!
+     * [mrdon] I dunno, are you planning to make protected getters/setters for every field?  I've found protected fields to be invaluable when extending frameworks (again, subscribing to the believe we should let the user do what they want and not artifically restrict them).  I do wish you could declare the fields _only_ available to subclasses and not to the whole package.
  
    1. Rename `ActionInvocation` to `Invocation` or `Request`. Shorter is better.
  
@@ -92, +98 @@

    1. Is `TextProvider` a good name? The JDK refers to these as "messages" everywhere.
  
      * [plightbo] The name doesn't bother me, but the implementation is far too complex. I would propose we cut it down from all the crazy package/interface/model-driven stuff we have now to a simple i18n RB loading approach that uses 0 or more global bundles as well as bundles associated with the _view_ (NOT the action, since that never made much sense).
+     * [mrdon] I'd like to see this re-evaluated as well.   For one, I want to make it easier to provide an impl that gets messages from a Struts bundle, which really isn't possible now.
  
    1. Come up with a clean way to separate "view" actions from "update" actions. For example, we might have `view()` and `update()` methods in the same action class. Right now XWork has `MethodFilterInterceptor`, but that's not a very clean solution. Do we want validation or the `DefaultWorkflowInterceptor` to run for the `view()` method? One solution is separate interceptor stacks, but it would be nice if there were 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... 
  
@@ -106, +113 @@

    1. On the OGNL value stack `#request` refers to request attributes and `#parameters` refers to the parameters. We could rename these `#request` for request parameters and `#requestAttributes` for request attributes.
  
      * [jcarreira] That one always has been confusing...
+     * [mrdon] Or #requestScope keeping in line with JSTL/JSP
  
    1. Warnings and failing silently is not the best practice. For example, if we can't find a method in a given action class, blow up ASAP (at load time). We shouldn't be fuzzy and do stuff like try to find a method named `doXxx()` when we can't find a method named `xxx()` (WebWork had backward compatibility restrictions, but we don't need to perpetuate them).
  
      * [plightbo] +1, everything should be much more proactive about failing fast and failing with a nice error message.
+     * [mrdon] Agreed, and we have done a lot of work lately on this issue.
  
    1. Add better support for file uploads.
  
@@ -120, +129 @@

    1. Don't eat/wrap exceptions. Throw them through to the container. Don't eat exceptions that occur in getters.
  
      * [plightbo] We're in serious need of a cleanup here.
+     * [mrdon] Again, changes have been made but much more to be done
  
    1. Modify `ParametersInterceptor` to sort parameter names by depth (using bucket sort) and then map them in that order (shallowest first), so we can create objects and then map fields to those objects in the same action invocation without hacks like applying the `ParametersInterceptor` twice or chaining.
  
@@ -159, +169 @@

    * [crazybob] +1 for JDK 1.5 since it came out. I have a lot of code I could contribute which depends on the new concurrency libraries, etc.
    * [MJ] With JDK 1.5 as a requirement for SAF2-based projects, users may be inclined to take a look at [http://stripes.mc4j.org/confluence/display/stripes/Home Stripes] first. It is compact, it features event-dispatching, built-in validation and conversion, Action and !ActionForm being one entity, and it allows to forgo XML config files by using annotations. The last feature alone is worth sack o'gold. If SAF2 is going to require JDK 1.5, it should allow XML-free configuration, at least for simple use cases.
    * [plightbo] I use JDK 1.5 as well. I think we really should think about doing this. Non-1.5 options exist (WebWork 2.2 and Struts 1.3), and we're really at a close point where it is no longer unreasonable to require 1.5 (1.6 will have been out for almost 6 months by the time we hit a final release).
+   * [mrdon] I'd like to use Java 5, but use retroweaver to continue to support 1.4.  If we keep XML in addition to annotations, that should be very doable.
  

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org