DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUGĀ·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38374>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED ANDĀ·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=38374





------- Additional Comments From [EMAIL PROTECTED]  2006-04-05 21:32 -------
(In reply to comment #37)
> Our class SaveAction performs a forward to a EditAction...

I use similar pattern, but I employ redirect, not forward. With new request
everything comes sparkling clean. But this is a religious topic and one has to
use session to save data between requests, or append it as a payload to
redirected URL.

Another religious topic is action chaining smell, but contrary to many, I am not
against it.

> ...when isCancelled() is true. 
> Because the CANCEL_KEY is still in the request
> an InvalidCancelException() is thrown before we get to the new EditAction.
> It's ugly for EditAction to know who could possibly forward to it,
> So placing a 
>       <set-property property="cancellable" value="true"/>
> In all the EditActions mappings seems a little confusing and non-intuative,

I never liked "cancellable" property at the first place because it is intrusive
(I guess I should have voiced my position on this.) Who said that Hollywood
principle is a good thing? If I want to know something I'll ask. If I need
something to be done, I will send a message to the appropriate object (fancy
old-fashioned way to say that I will call a damn method). Hollywood principle
sucks, and this particular incident stresses this out one more time.

> There needs to be a clean way to cleanup this attribute, and I beleive 
> it should be the SaveAction() in this case.
> 
> One possible changes would be to provide a method similar to isValidToken()
> that explicitly clears the CANCEL_KEY attribute 
>   protected boolean isCancelled(HttpServletRequest request, boolean remove) {
>        boolean cancelled = isCancelled(request);
>        if (cancelled && remove) {
>          request.removeAttribute(Globals.CANCEL_KEY);
>        }
>        return (cancelled);
>    }

It is a bad practice to have side effects in getters, it is even worse to have
getters that are non-idempotent.

> Another method that is a little more involved would be to add an attribute
> in the controller declaration that says to clean up the CANCEL_KEY before 
> performing a forward.

This is better but still a hassle. I don't see how simple

  request.removeAttribute(Globals.CANCEL_KEY);

is worse than having another attribute in config file (and another update to
DTD). Struts 1.x is deeply penetrated with references to request/response
objects, so let's not pretend that it is a Servlet API virgin.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to