MappingDispatchAction dilema

[Stephen Houston <st...@egeekworld.com>]

Hi,

We have a bit of dilemma on our project, and I want to get some feedback
from other Struts developers on whether or not what we are thinking of
doing goes against Struts best practices. We are using Struts 1.2 and
all our Action inherit from a CommonAction which extends
MappingDispatchAction, with each of them providing a show (forwards to
the JSP) and submit (updates the model) methods. We have noticed that
each of our show method has a lot in common with all the other show
methods, so are considering what the best way would be to factor this
into the CommonAction.

Currently we have something akin to this in each show method in each
Action

public ActionForward show(ActionMapping mapping, 
	ActionForm form, 
	HttpServletRequest request, 
	HttpServletResponse reponse) throws Exception {

	log.debug(">> show");

	MapBackedForm bean = (MapBackedForm) form;

	// Audit logging, log the details of the user and their
requested action

	// Security check, check they are allowed to execute the action

	// Set up page header details from model

	String destination = "success";
	// Execute specific action code which results in destination
being updated

	log.debug("<< show");

	return mapping.findForward(destination);

}

We are considering making changes to our CommonAction so that the
MappingDispatchAction would execute the show method on it, and it would
in turn call a method on our subclass, e.g.

CommonAction.java
public final ActionForward show(ActionMapping mapping,
	ActionForm form,
	HttpServletRequest request,
	HttpServletResponse response) throws Exception {
	
	log.debug(">> show");

	MapBackedForm bean = (MapBackedForm) form;

	// Audit logging, log the details of the user and their
requested action

	// Security check, check they are allowed to execute the action

	// Set up page header details from model

	// Execute specific action code
	String destination = show(bean, session);

	log.debug("<< show");

	return mapping.findForward(destination);

}

public abstract String show(MapBackedForm form, HttpSession session)
throws Exception;

Then each action would implement our 2 parameter show method, and return
a String which could be passed to the ActionMapping by the CommonAction
superclass. This would significantly simplify the code in our Action
classes, reducing repetition between each one, and ensuring that they
each follow all the required auditing and security checks.

I'm wondering if it's considered a bad idea to have the
MappingDispatchAction's dispatch method invoke a call on the superclass
which in turn delegates to the subclass after executing all the common
stuff, because all our subclasses would have a different method
signature than that which is standard in Struts. Or is it acceptable to
do this because our CommonAction class will insulate us from any
potential changes to Struts which would affect this method signature. 

Has anyone else done anything similar, or are we totally missing the
point on this one and there's a much neater way of doing it?

Note: we would also change our submit method so that it followed the new
signature above. It shares the same duplication of code.

Thanks

Steve

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

Re: MappingDispatchAction dilema

[Kishore Senji <ks...@gmail.com>]

> Then each action would implement our 2 parameter show method, and return
> a String which could be passed to the ActionMapping by the CommonAction
> superclass. This would significantly simplify the code in our Action
> classes, reducing repetition between each one, and ensuring that they
> each follow all the required auditing and security checks.

Since your submit also shares the same duplicate code. Instead of
having "show", "submit" and other methods having the same code in the
CommonAction, you can override the "execute" method.

> 
> I'm wondering if it's considered a bad idea to have the
> MappingDispatchAction's dispatch method invoke a call on the superclass
> which in turn delegates to the subclass after executing all the common
> stuff, because all our subclasses would have a different method
> signature than that which is standard in Struts. 

Well, delegating some work to the superclass method and then calling
the subclass method is is a good practice. But I would be careful in
changing the signature of the method. What if the subclass method
requires some special processing to do based on the request
parameters? So, I would like to keep the signature exactly the same.

Or is it acceptable to
> do this because our CommonAction class will insulate us from any
> potential changes to Struts which would affect this method signature.
> 

Well, if Struts method signature changes, you would have to change the
super class show method anyway.

My version of your Common Action would be
public class CommonAction extends MappingDispatchAction{

public static final String SUCCESS = "success";
public static final String FAILURE = "failure";

  public ActionForward execute(ActionMapping mapping, 
                               ActionForm form, 
                               HttpServletRequest request, 
                               HttpServletResponse response){
    
    log.debug(">> " + mapping.getParameter());
    
    // Audit logging, log the details of the user and their requested action
    // Security check, check they are allowed to execute the action
    // Set up page header details from model
    // Execute specific action code
    
    ActionForward forward = super.execute(mapping, form, request, response);

    log.debug("<< " + mapping.getParameter());
 
    return forward;
  }
}

and the subclasses of the common action would have show and update
methods with the same signature as execute

  public ActionForward show(ActionMapping mapping, ActionForm form,
HttpServletRequest request, HttpServletResponse response){
    // show logic
    return mapping.findForward(SUCCESS);
  }
  public ActionForward update(ActionMapping mapping, ActionForm form,
HttpServletRequest request, HttpServletResponse response){
    // update logic
    return mapping.findForward(SUCCESS);
  }

Now, I put common code in the execute of the CommonAction. If there
are any differences in the common code b/n different methods (show,
update), you can have show & update methods in the CommonAction and
can have the subclasses overriding those methods and calling
super.execute().

Thanks,
Kishore Senji.

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