Request for new functionality..

[Kevin Duffey <kd...@buymedia.com>]

Hi all,

Mostly to Craig, but would like to see some comments too.

I find myself putting the same bit of code in every Actin perform() method.
That is, to allow one Action to handle multiple forms, using the "command"
pattern, in every Action I am doing something like this:

perform(..)
{
  int command = COMMAND_DEFAULT;

  try
  {
    command = Integer.parseInt( request.getAttribute("command") );
  }
  catch(Exception e)
  {
    // do nothing..command already is set to COMMAND_DEFAULT
  }

  String forwardStr = null;

  switch( command )
  {
    case COMMAND_DEFAULT: forwardStr = executeCommandDefault( form,
request );
         break;
    case COMMAND_SOMETHING_ELSE : forwardStr =
xecuteCommandSomethingElse( form, request );
         break;

   ...
  }

  return mapping.findForward( forwardStr );
}


What I suggest is that the Struts framework pass to the perform() method as
the first attribute an int value of the command pattern value. Then, have
the peorm method return a String type, which in the Controller servlet is
used to do the mapping.findForward(returnString);

The name of the "command" parameter would be defineable as an init parameter
in web.xml.

What I have done is created a DefaultAction which implements the perform()
method. I descend all Action classes from this action class, which
implements Action.

I have something like the above, with one added step..:


perform(..)
{
  int command = COMMAND_DEFAULT;
  try
  {
    command = Integer.parseInt( request.getParameter("command") );
  }
  catch(Exception e)
  {
  }

  return mapping.findForward( dispatch( command, servlet, mapping, form,
request, response ) )
}

then I have:

abstract String dispatch( int command, ActionServlet servlet, ActionMapping
mapping, ActionForm form, HttpServletRequest request, HttpServletResponse
response);

Now, all descendant classes implement the dispatch() method which returns a
String that represents the mapping in the action.xml file.

The reason I like this approach is because I no longer have to get the
command for each perform() method, and I no longer have to forward in each
action class...but I do have the ability to if I want to. Every action class
automatically gets the command paramter if it exists, or COMMAND_DEFAULT if
not, and it simply returns the name of the String which is mapped to a JSP
page to forward to.

While what I did is fine..and it works, it might be a feature many would
like to see..as it seems the "command" pattern is a common approach to
handling multiple different requests with one action.

What does anyone/everyone think?

RE: Request for new functionality..

[Kevin Duffey <kd...@buymedia.com>]

Hi,

> My personal preference is to be pretty cautious about adding
> global features
> (and especially where it breaks the code that is starting to be
> built on Struts)
> unless it is really something that everyone needs -- like the incompatible
> change we did to return ActionForward instead of void from the
> perform() method.

I tend to agree..actually..I had to rework about 200 lines of code for that
change! But, it was for the better, and this too I think is for the better
IF people want to use it. However, what you may like to do is add in a class
that implements Action..call it DefaultAction like I did. Have it implement
the Action interface..infact, I should just do it and send you the file..if
you want it. Then, it doesn't break the code..but it gives the Struts users
the ability to extend the DefaultAction class, instead of implement the
Action interface. This way, they get a default behavior that allows the
getting of the command parameter, AND forwarding to the proper page via the
mapping, without having to be forced to use it.

> One easy way to deal with this pattern in the current
> architecture is to extend
> the ActionMapping classes you use with a "command" property.
> Even though you
> might be calling the same Action class ultimately, you would have
> a separate
> ActionMapping for each of them (so you can change your mind later about
> combining versus not combining them and not have to change the
> user interface).
>
> The way that I dealt with this concept in the sample application
> was a hidden
> "action" field in the input form (and the corresponding form bean
> so that it
> gets populated automatically -- and you can even validate it!) so
> that I can
> access it like any other bean property.  That way, you can use
> the "command"
> pattern where you need it, and not where you don't.

True..but I don't think its quite the same. What I am proposing is a pretty
small bit of code that implements the perform method and calls a Dispatch
method that all descendant action classes would implement (abstract in
DefaultAction class). They simply rename their current perform() methods to
add in the int parameter, and return a String instead of Forward..then
return a string. But for new classes, it would be a little faster by just
extending DefaultAction and return a String that is used as the lookup to
forward to a page.

At any rate..here is the code if anyone wants it, or if Craig wants to add
it in to the Struts framework:


DefaultAction.java:

package org.apache.struts.action;

import javax.servlet.*;
import javax.servlet.http.*;

import java.io.*;
import java.sql.*;

import org.apache.struts.action.*;

public abstract class DefaultAction
  implements Action, Serializable
{
  public static final int             COMMAND_DEFAULT     = 0;

  public DefaultAction()
  {
  }


  public ActionForward perform(ActionServlet servlet, ActionMapping mapping,
ActionForm form, HttpServletRequest request, HttpServletResponse response)
    throws IOException, ServletException
  {
    int command;

    try
    {
      command = Integer.parseInt( request.getParameter("command") );
    }
    catch(Exception e)
    {
      command = COMMAND_DEFAULT;
    }

    return mapping.findForward( dispatch( command, servlet, mapping, form,
request, response ) );
  }


  public abstract String dispatch( int command, ActionServlet servlet,
ActionMapping mapping, ActionForm form, HttpServletRequest request,
HttpServletResponse response)
    throws IOException, ServletException;
}




Thats really all there is to it..but I also have a number of methods that
descendant actions will use, such as getting the session key name for the
"logic" session class object that correspond to a given action, and a few
others (logging, etc).

Now, a descendant class would look something like this:


import javax.servlet.*;
import javax.servlet.http.*;

import java.io.*;
import java.sql.*;

import org.apache.struts.action.*;

public final class MyAction
  extends DefaultAction
{
  public static final int               COMMAND_DO_SOMETHING = 100;

  public String dispatch(int command, ActionServlet servlet, ActionMapping
mapping, ActionForm form, HttpServletRequest request, HttpServletResponse
response)
    throws IOException, ServletException
  {
    switch (command)
    {
      case COMMAND_DEFAULT       : return executeCommandDefault( form,
request );
      case COMMAND_DO_SOMETHING  : return executeCommandDoSomething( form,
request );
    }

    return "";
  }

  public String executeCommandDefault( ActionForm form, HttpServletRequest
request )
  {
    return "";
  }

  public String executeCommandDefault( ActionForm form, HttpServletRequest
request )
  {
    return "";
  }
}


The above is a stub extended class, but it should be enough to get the point
across. What I like is that I just return the String type, and the
forwarding is done for me in one place..instead of having to put it in every
action. However..you always have the ability to forward if you want. Same
for the command value..its done for you in one place.

Anyways..for whats it worth..if its added, fine..if not, no biggee. Just
figured I'd see if something I am using that I find helpful would be helpful
to others as well.

RE: Request for new functionality..

[Kevin Duffey <kd...@buymedia.com>]

Hi,

> My personal preference is to be pretty cautious about adding
> global features
> (and especially where it breaks the code that is starting to be
> built on Struts)
> unless it is really something that everyone needs -- like the incompatible
> change we did to return ActionForward instead of void from the
> perform() method.

I tend to agree..actually..I had to rework about 200 lines of code for that
change! But, it was for the better, and this too I think is for the better
IF people want to use it. However, what you may like to do is add in a class
that implements Action..call it DefaultAction like I did. Have it implement
the Action interface..infact, I should just do it and send you the file..if
you want it. Then, it doesn't break the code..but it gives the Struts users
the ability to extend the DefaultAction class, instead of implement the
Action interface. This way, they get a default behavior that allows the
getting of the command parameter, AND forwarding to the proper page via the
mapping, without having to be forced to use it.

> One easy way to deal with this pattern in the current
> architecture is to extend
> the ActionMapping classes you use with a "command" property.
> Even though you
> might be calling the same Action class ultimately, you would have
> a separate
> ActionMapping for each of them (so you can change your mind later about
> combining versus not combining them and not have to change the
> user interface).
>
> The way that I dealt with this concept in the sample application
> was a hidden
> "action" field in the input form (and the corresponding form bean
> so that it
> gets populated automatically -- and you can even validate it!) so
> that I can
> access it like any other bean property.  That way, you can use
> the "command"
> pattern where you need it, and not where you don't.

True..but I don't think its quite the same. What I am proposing is a pretty
small bit of code that implements the perform method and calls a Dispatch
method that all descendant action classes would implement (abstract in
DefaultAction class). They simply rename their current perform() methods to
add in the int parameter, and return a String instead of Forward..then
return a string. But for new classes, it would be a little faster by just
extending DefaultAction and return a String that is used as the lookup to
forward to a page.

At any rate..here is the code if anyone wants it, or if Craig wants to add
it in to the Struts framework:


DefaultAction.java:

package org.apache.struts.action;

import javax.servlet.*;
import javax.servlet.http.*;

import java.io.*;
import java.sql.*;

import org.apache.struts.action.*;

public abstract class DefaultAction
  implements Action, Serializable
{
  public static final int             COMMAND_DEFAULT     = 0;

  public DefaultAction()
  {
  }


  public ActionForward perform(ActionServlet servlet, ActionMapping mapping,
ActionForm form, HttpServletRequest request, HttpServletResponse response)
    throws IOException, ServletException
  {
    int command;

    try
    {
      command = Integer.parseInt( request.getParameter("command") );
    }
    catch(Exception e)
    {
      command = COMMAND_DEFAULT;
    }

    return mapping.findForward( dispatch( command, servlet, mapping, form,
request, response ) );
  }


  public abstract String dispatch( int command, ActionServlet servlet,
ActionMapping mapping, ActionForm form, HttpServletRequest request,
HttpServletResponse response)
    throws IOException, ServletException;
}




Thats really all there is to it..but I also have a number of methods that
descendant actions will use, such as getting the session key name for the
"logic" session class object that correspond to a given action, and a few
others (logging, etc).

Now, a descendant class would look something like this:


import javax.servlet.*;
import javax.servlet.http.*;

import java.io.*;
import java.sql.*;

import org.apache.struts.action.*;

public final class MyAction
  extends DefaultAction
{
  public static final int               COMMAND_DO_SOMETHING = 100;

  public String dispatch(int command, ActionServlet servlet, ActionMapping
mapping, ActionForm form, HttpServletRequest request, HttpServletResponse
response)
    throws IOException, ServletException
  {
    switch (command)
    {
      case COMMAND_DEFAULT       : return executeCommandDefault( form,
request );
      case COMMAND_DO_SOMETHING  : return executeCommandDoSomething( form,
request );
    }

    return "";
  }

  public String executeCommandDefault( ActionForm form, HttpServletRequest
request )
  {
    return "";
  }

  public String executeCommandDefault( ActionForm form, HttpServletRequest
request )
  {
    return "";
  }
}


The above is a stub extended class, but it should be enough to get the point
across. What I like is that I just return the String type, and the
forwarding is done for me in one place..instead of having to put it in every
action. However..you always have the ability to forward if you want. Same
for the command value..its done for you in one place.

Anyways..for whats it worth..if its added, fine..if not, no biggee. Just
figured I'd see if something I am using that I find helpful would be helpful
to others as well.

Re: Request for new functionality..

["Craig R. McClanahan" <Cr...@eng.sun.com>]

Kevin Duffey wrote:

> Hi all,
>
> Mostly to Craig, but would like to see some comments too.
>
> I find myself putting the same bit of code in every Actin perform() method.
> That is, to allow one Action to handle multiple forms, using the "command"
> pattern, in every Action I am doing something like this:
>

My personal preference is to be pretty cautious about adding global features
(and especially where it breaks the code that is starting to be built on Struts)
unless it is really something that everyone needs -- like the incompatible
change we did to return ActionForward instead of void from the perform() method.

One easy way to deal with this pattern in the current architecture is to extend
the ActionMapping classes you use with a "command" property.  Even though you
might be calling the same Action class ultimately, you would have a separate
ActionMapping for each of them (so you can change your mind later about
combining versus not combining them and not have to change the user interface).

The way that I dealt with this concept in the sample application was a hidden
"action" field in the input form (and the corresponding form bean so that it
gets populated automatically -- and you can even validate it!) so that I can
access it like any other bean property.  That way, you can use the "command"
pattern where you need it, and not where you don't.

Craig McClanahan

Re: Request for new functionality..

["Craig R. McClanahan" <Cr...@eng.sun.com>]

Kevin Duffey wrote:

> Hi all,
>
> Mostly to Craig, but would like to see some comments too.
>
> I find myself putting the same bit of code in every Actin perform() method.
> That is, to allow one Action to handle multiple forms, using the "command"
> pattern, in every Action I am doing something like this:
>

My personal preference is to be pretty cautious about adding global features
(and especially where it breaks the code that is starting to be built on Struts)
unless it is really something that everyone needs -- like the incompatible
change we did to return ActionForward instead of void from the perform() method.

One easy way to deal with this pattern in the current architecture is to extend
the ActionMapping classes you use with a "command" property.  Even though you
might be calling the same Action class ultimately, you would have a separate
ActionMapping for each of them (so you can change your mind later about
combining versus not combining them and not have to change the user interface).

The way that I dealt with this concept in the sample application was a hidden
"action" field in the input form (and the corresponding form bean so that it
gets populated automatically -- and you can even validate it!) so that I can
access it like any other bean property.  That way, you can use the "command"
pattern where you need it, and not where you don't.

Craig McClanahan