struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Apache Wiki <wikidi...@apache.org>
Subject [Struts Wiki] Update of "RoughSpots" by JasonCarreira
Date Wed, 19 Apr 2006 21:09:06 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 JasonCarreira:
http://wiki.apache.org/struts/RoughSpots

------------------------------------------------------------------------------
      * [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."
      * [crazybob] I don't think we should make everything final or anything, but I do think
we should differentiate between the published API and the implementation through package organization,
excluding stuff from the Javadocs, etc. Users can know that we won't change the published
API out from under them, but that if they depend on implementation classes, we may break them
(reluctantly). It's the polite thing to do, and it frees us up to refactor and improve our
implementation.
+     * [jcarreira] +1 published public vs. private APIs are a good thing, and a contract
with the user of what can change. 
  
    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.
  
@@ -64, +65 @@

      * [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.
+     * [jcarreira] +1 to Patrick's point... we may need to introduce some more advanced *Aware
interfaces, though, to give people access to the guts if they really want it.
  
    1. Is `ValidationAware` a good name? Perhaps `Errors` or `ErrorList` would be a better
name.
  
@@ -74, +76 @@

      * [jcarreira] It __is__ well specified... There are some things that the ActionProxy
/ ActionInvocation let you do that a merged one doesn't... for instance easily knowing when
you're done :-)
      * [crazybob] Does "specified" == "documented"? Can you elaborate on "easily knowing
when you're done" and why we can't address that use case with one interface? We should expose
the user to one interface: `Invocation`. We can have as many objects as we like when it comes
to the internal implementation.
      * [plightbo] Big +1. I would add the ValueStack to this list as well. Let's have one
object for each invocation, and also make "sub invocations" (chaining or ww:action calls)
a first class citizen through child/parent invocation relationships.
+     * [jcarreira] Let's have an interactive design session on this... I'm not sure it's
as easy as you think to merge them all into one (although behind one interface should be doable).
  
    1. Should `ActionInvocation.getResult()` recurse over chain results? Maybe we should have
two methods? `getResult()` and `getFinalResult()`. Is there a good use case for this?
  
@@ -91, +94 @@

      * [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.
      * [crazybob] Absolutely, methods instead of fields. Methods enable us to hook access
if need be, change the underlying implementation, manage thread safety, protect our own code
against unexpected conditions/states, etc. Fields are OK within a package, but when it comes
to public APIs, better safe than sorry a year from now.
+     * [jcarreira] Oops, I was translating "fields" -> "properties" and thinking of getters
and setters.
  
    1. Rename `ActionInvocation` to `Invocation` or `Request`. Shorter is better.
  
@@ -101, +105 @@

  
      * [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.
+     * [jcarreira] +1 to simplifying. We started with lots of separate resource bundles for
per-action texts, but we're moving to one resource bundle per module. It's too much hassle.
  
    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... 
  
@@ -146, +151 @@

  
    1. Use a lot more conventions with easy overrides.
    1. Don't dismiss XML entirely - annotations are nice but currently can't be HotSwapped
(due to a bug in the JDK). For any configuration, we should read in the following order: XML,
annotations, convention.
+      * [jcarreira] Shouldn't annotations be the default, and XML be the override?
    1. Fail fast with detailed error messages, ideally ones that show you what you did wrong
and what you should to.
    1. Address the confusing issue of the validation/workflow lifecycle and different methods
(this is mentioned in more detail above, but it is something that is problematic). Right now
we sort of hack it by making the "input" method a special case in webwork-default.xml.
+      * [jcarreira] +1 : Carlos at G**gle had some good ideas for this... basically stuff
like if your action method is foo() then you'd have prepareFoo() and validateFoo(), but then
I added that the prepare() and validate() methods should be the defaults that we call for
all action methods.
    1. Don't encourage lots of interceptor stacks. Ideally the normal user should never need
to deal with them. It is better to have a larger stack that has optional features that could
be turned on through annotations or marker interfaces than to encourage users to build their
own stacks.
  
+      * [jcarreira] I think we should have some pre-defined ones for standard things: view
vs. CRUD vs. "action" -> do somthing that's not CRUD. We should then use annotations to
make it where you can declaratively associate a particular action method with a "stereotype"
which is mapped to an interceptor stack, etc.
  == Nice to haves ==
  
    1. Inheritance is not a good way to reuse code between actions. One work around is to
use the strategy pattern to swap in different implementations of interfaces like `ValidationAware`.
It would be nice if the framework had built-in support for mixins using cglib or Dynaop. For
example, instead of extending a class that implements `ValidationAware`, SAF could extend
an action class at runtime and implement the `ValidationAware` methods by delegating them
to another object (a mixin): {{{
@@ -165, +173 @@

      * [crazybob] It only has to be abstract if you want your action to be able to call methods
on the mixin without casting. If it doesn't need to call those methods, there's no need for
your action to explicitly implement that interface. You could also say `((ValidationAware)
this).addActionError()`. I personally don't mind making the action abstract. In IntelliJ,
you just make a mock class that extends your action and it will automatically generate stubs
for the methods.
      * [plightbo] As mentioned earlier, you might be able to do this by using the value stack.
For example, you could stick in a single "ValidationSupport" object at the bottom of the stack
and then all classes would add error messages there. By adding a simple *Aware interface,
actions could get a handle to that for adding error messages.
  
+     * [jcarreira] Very nice idea... +1 ... We already do something like that. There's always
a default TextProvider pushed onto the bottom of the stack for getting default texts... makes
localized texts work with Sitemesh, for instance, when you're not hitting an action. Of course,
that will be a problem when we're not using ThreadLocals. 
+ 
  == What JDK Version? ==
  
    * [jcarreira] We've been using JDK 1.5 on Tomcat 5+ for over a year... Everything we write
and wire together is using generics and annotations.

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


Mime
View raw message