[ 
https://issues.apache.org/struts/browse/STR-3009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_40350
 ] 

Michael Jouravlev commented on STR-3009:
----------------------------------------

> 1) Do not modify ForwardConfig. It's nothing but a mapping and does not
> indicate the type of mapping, so it's wrong to assume it is a forward 
> or redirect (although it generally always is).

It does indicate redirect/forward in the config file. An optional parameter 
would help to override this value.

> 2) Do not add parameter adding directly to ActionForward. Although
> ActionRedirect is a subclass of ActionForward, the hierarchy is wrong
> because a redirect is not a philosophical subset of a forward. They are
> just differing but similar technologies.

Agreed.

> 3) Create a new subclass between ActionMapping and ActionForward called
> ActionParameterized and place the functionality there. Just take the code
> in ActionRedirect and move it up the hierarchy.

This is what I wanted to do at first, calling it ActionTransfer in terms that 
transfer is generalization of forward and redirect.

But I think that sometimes too much OOP is too much. Here is the reasons why 
adding parameters to ActionMapping.findForward is simpler:

* Users often do not think about ActionForward objects, I believe that many 
automatically write code like return mapping.findForward("success") without 
much thinking what exactly is returned.
* Many users do not know that preconfigured mappings are frozen and can't be 
updated. Thy may be tempted to do so, especially considering that ActionMapping 
has methods for that. By passing parameters to findForward the framework will 
automatically decide whether to return a canned object or to create a new 
instance.
* to me the following

  return mapping.findForward("success", true, 
      new String[][] {{"objid",objid},{"mode","VIEW"}});

or

  return mapping.findForward("success", true, 
      new ActionParam(new String[][] {{"objid",objid},{"mode","VIEW"}}));

 is simpler than

     ActionRedirect redirect =
             new ActionRedirect(mapping.findForward("success"));
     redirect.addParameter("objid",objid);
     redirect.addParameter("mode","VIEW");
     return redirect;

But I don't care too much about this particular aspect of Struts to push my 
suggestion. ActionTransfer a.k.a. ActionParameterized will work for me too. 

> ActionRedirect from ForwardConfig not redirecting properly?
> -----------------------------------------------------------
>
>                 Key: STR-3009
>                 URL: https://issues.apache.org/struts/browse/STR-3009
>             Project: Struts 1
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.3.5, 1.3.6
>            Reporter: Jason Millard
>
> While migrating from Struts 1.2.9 to Struts 1.3.5, I noticed ActionRedirect 
> was no longer redirecting as expected. 
> In the Struts 1.2.9, in 
>   public ActionRedirect(ForwardConfig baseConfig) 
> setRedirect was always being set to TRUE, like it is in every other 
> constructor.
> However, as of Struts 1.3.5 it is now being configured as
>  setRedirect(baseConfig.getRedirect());
> This seems to defeat the purpose of a ActionRedirect if you are trying to 
> generate one from a
>   mapping.findForward();

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to