aspose file tools*
The moose likes Struts and the fly likes [Struts Recipes] Action chaining: recipe for disaster? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Spring in Action this week in the Spring forum!
JavaRanch » Java Forums » Frameworks » Struts
Bookmark "[Struts Recipes] Action chaining: recipe for disaster?" Watch "[Struts Recipes] Action chaining: recipe for disaster?" New topic
Author

[Struts Recipes] Action chaining: recipe for disaster?

Junilu Lacar
Bartender

Joined: Feb 26, 2001
Posts: 4742
    
    7

Action chaining is when you forward processing from one action to another. I have always tried to discourage the practice on the Struts projects I've been on because I have seen too many bad things happen because of it.

My take on Action chaining in Struts is that it is often the smell of "feature envy". I will usually recommend a refactoring that involves moving the "envied" code into a helper class and changing the two actions so that they delegate to the Helper class instead of one Action delegating to the other. In most cases it works, but sometimes it doesn't. I haven't really sat down long enough to analyze in detail what particular circumstances make the elimination of Action chaining difficult. All I know is that when I see Action chaining happening, the code is usually complex, convoluted, brittle and/or fraught with bugs.

Do you have any recipes in your book/bag of tricks for eliminating or mitigating the ill effects of Action chaining?

Also, what are your thoughts about Action chaining in Struts: good or bad?

TIA


Junilu - [How to Ask Questions] [How to Answer Questions]
Srikanth Shenoy
author
Ranch Hand

Joined: Jan 24, 2004
Posts: 184
Hi Junilu,

Sorry for replying to this question meant to the authors.

There is lot of misconception that action chaining is evil all the times.
Action Chaining is not bad all the time. There are cases where Action Chaining results in extremely good design.

Consider an example: A page shows a list of loans with option to delete loans one at a time. After deletion, the same loan list is shown again. If the user is forwarded directly to the List JSP after deletion, then the task of creating the loan list is left to the JSP. That is a bad design. Applying pure MVC means invoking the logic to fetch the List again. Instead, Action chaining saves the day here.

Another scenario: Using the action mapping of self as the input attribute is a special case of action chaining and comes handy when a lot or preprocessing is needed to show a page, irrespective of whether page is shown for the first time in the normal way or because of validation errors.

Then there are scenarios where action chaining is a bad idea. If the chaining is used for linking several units of business logic one after the other, it is better to do this in the business tier. If this is one of your goals, then use a session ejb method as a fa�ade to hide the execution of fine-grained business logic snippets as one unit instead of chaining actions.

If you are having more than two actions in the chain, chances are that you are trying to do business logic by chaining Actions. A strict no-no. Nada.

My 2 cents on advantages of Action Chaining. With that I humbly let the authors take over.


Srikanth Shenoy
Author of Struts Survival Guide : Basics to Best Practices
Junilu Lacar
Bartender

Joined: Feb 26, 2001
Posts: 4742
    
    7

Originally posted by Srikanth Shenoy:

Sorry for replying to this question meant to the authors.

I'm glad you answered. If anyone other than the authors has something to say, by all means say it.

Consider an example: A page shows a list of loans with option to delete loans one at a time. After deletion, the same loan list is shown again. If the user is forwarded directly to the List JSP after deletion, then the task of creating the loan list is left to the JSP. That is a bad design. Applying pure MVC means invoking the logic to fetch the List again. Instead, Action chaining saves the day here.

Still not seeing it. I still smell feature envy.

BuildLoanListAction --> loanlist.jsp --> DeleteLoanAction --> BuildLoanListAction --> loanlist.jsp.

Here's how I'd refactor:

  • Take code from BuildLoanListAction and put into LoanListBuilder helper class.
  • BuildLoanListAction uses LoanListBuilder to get a list of loans, forwards to loanlist.jsp
  • DeleteLoanAction uses LoanListBuilder to get a list of loans,

  • forwards to loanlist.jsp


    DeleteLoanAction is now no longer dependent on BuildLoanListAction. If, for some reason, BuildLoanListAction is changed, I wouldn't have to go and test DeleteLoanAction too. LoanListBuilder can be reused by any number of classes, including non-Action classes. This, to me, seems like a better design. Non-Action classes may be able to reuse BuildLoanListAction but because it is a Control layer class, the coupling may not be as natural as with LoanListBuilder, a POJO that's not layer-specific.

    Also, it is a common practice to lump CRUD operations into one Action class. Chaining to such a class often results in a lot of unnatural conditional logic being written to ensure the proper routing of the request. This just adds a lot of unnecessary complexity to the Action class code.
    [ January 27, 2005: Message edited by: Junilu Lacar ]
    Srikanth Shenoy
    author
    Ranch Hand

    Joined: Jan 24, 2004
    Posts: 184

    BuildLoanListAction --> loanlist.jsp --> DeleteLoanAction --> BuildLoanListAction --> loanlist.jsp.

    Here's how I'd refactor:

  • Take code from BuildLoanListAction and put into LoanListBuilder helper class.
  • BuildLoanListAction uses LoanListBuilder to get a list of loans, forwards to loanlist.jsp
  • DeleteLoanAction uses LoanListBuilder to get a list of loans,

  • forwards to loanlist.jsp



    It's my recent work speaking, but most of the times for me, it is more than fetch loan and then forward to loanlist.jsp. The logic whether to show delete button in the loanlist.jsp and some other minor things are calculated before showing the jsp itself. I prefer to think that building loan list and other related stuff (like delede button logic) plus showing the loanlist.jsp as an atomic operation and handle it in BuildLoanListAction. That way I dont have to do it in JSP or LoanListBuilder.

    I guess both of us may be right in this case.


    Also, it is a common practice to lump CRUD operations into one Action class. Chaining to such a class often results in a lot of unnatural conditional logic being written to ensure the proper routing of the request. This just adds a lot of unnecessary complexity to the Action class code.


    I agree this one is not a candidate for chaining.

    Thanks,
    Srikanth
    Karthik Guru
    Ranch Hand

    Joined: Mar 06, 2001
    Posts: 1209
    Hi Junilu,

    I'm not very clear. Do correct me. Cant we put the actual deletion and retreival logic in the helper class and still do action chaining? I mean the Delete and Retreival action still use the helper to execute business logic. The DeleteAction does what it is supposed to ie delete the entity and the Retreival action continues to do its job of 'getting'.
    When you say complex code, are you implying that the action that features earlier in the chain might have to populate some additional context information for the one that follows it?
    Kevin Judd
    Greenhorn

    Joined: Dec 02, 2004
    Posts: 9
    Junilu and Srikanth,

    Thanks for some substantive dialogue on an advanced topic.

    I don't propose a solution, but find this a perplexing issue and an ongoing dilemna.

    In my applications, the ListBuilder Action is itself the result of a query to allow the user to narrow down the list of options to list. Thus, I end up with a set of select list parameters that must follow the action and be invoked when building the select list (and perhaps be re-queried). I tried keeping all of this stuff in Helper classes (to reduce chaining) but came to the conclusion that chaining allowed for more atomic actions which are a bit easier to string together using the Action Classes, and fit better into the Struts paradigm of keeping as much stuff in the config files as possible.

    The caveat, though, is that I end up with a snake pit of disparate actions in my config file, and there is no apparent structure to the actions (except that which I can write in with the action namings.)

    So, I see what Junilu means by "complex, brittle, convoluted" code.

    Then, after building this thing, I want to add a Confirmation page that is not dependant on JavaScript. Agghhh..


    Well, sorry. My note here is more cathartic than informative.


    Kevin Judd, SCJP
    Jason Menard
    Sheriff

    Joined: Nov 09, 2000
    Posts: 6450
    Another thing that is happening here is tier leakage. Specifically, too much business logic often ends up in the Actions. Actions should actually be very small. If your Action is too large than that is a pretty good indication of tier leakage. Another indication of tier leackage is often Action chaining.

    When we are developing our applications, we should be able to pretty much rip out the view and the control and replace them if needed with some other technology. If you program with this in mind, you will do a much better job at separating the layers and keeping logic where it needs to be.

    A good way to help move in this direction is to have business classes with methods that pretty much resolve an entire use case as far as the business logic is concerned, presenting back to the control any data that needs to be forwarded to the view. The view (JSPs) should not really need to have much in the way of logic, and the control (Actions) really shouldn't have much in the way of logic.

    Looking at Srikanth's example, I might have a business object called LoanServices. This class acts as a facade for the rest of my API. Each public method in the service classes will probably each resolve a use case. The use case "Delete Loan" probably goes something like this:

    1. Given a list of loans, user selects a loan for deletion.
    2. System deletes selected loan.
    3. System presents user with a new list of loans.

    Cutting to the heart of the matter, here's a solution:

    LoanInfo --> summary Loan information

    LoanList --> contains a List of LoanInfo objects

    loanList.jsp --> iterates over a LoanList object to display the component LoanInfo objects.

    DeleteLoanAction
    ----------------
    1. long loanId = get parameter from request
    2. LoanList loanList = LoanServices.deleteLoan(loanId)
    3. request.setAttribute(Globals.LOAN_LIST_KEY, loanList)
    4. return mapping.findForward("success")

    LoanServices.deleteLoan(long loanId)
    ------------------------------------
    1. LoanDAO.delete(loanId)
    2. LoanList loanList = LoanDAO.getLoanList()
    3. return loanList

    LoanDAO --> contains methods for performing operations on loan data. This is where the database access stuff is kept.

    You'll notice that the LoanServices.deleteLoan method encapsulates all the business logic required to satisfy the use case. You could drop your LoanServices object in a Swing app and everything would work just as well. Just as importantly, you've got all the critical business logic being handled by business classes, as you should. If you architect your apps in a manner such as I've desribed, you should very rarely have to consider something like Action chaining.
    Junilu Lacar
    Bartender

    Joined: Feb 26, 2001
    Posts: 4742
        
        7

    Originally posted by Karthik Guru:
    Cant we put the actual deletion and retreival logic in the helper class and still do action chaining? I mean the Delete and Retreival action still use the helper to execute business logic. The DeleteAction does what it is supposed to ie delete the entity and the Retreival action continues to do its job of 'getting'.
    When you say complex code, are you implying that the action that features earlier in the chain might have to populate some additional context information for the one that follows it?


    The complexity and convolution usually ends up in the chained action, the one that has the features being envied by other Actions. Typically, I'll see this kind of code (note: not actual code, just trying to show the essence):



    Yucky, but not uncommon in my experience. YMMV. Gets even more yucky if the upstream actions handle several types of requests, then you'll see code like:



    Eeeew. I know that some programmers recognize the ugliness of this because I have seen them write other helper classes to try to "clean up" this kind of logic such as a "WebFlowDirector", which encapsulates the control path that needs to be followed for a certain request. This class is passed around in the request context and is queried by each Action in the chain to determine where the request should go next. It still feels clunky and overly complex to me though.

    Mind you, I don't write the kind of code shown above. I have just seen others do it often enough that I tend to discourage the practice that leads them to writing this kind of code in the first place, namely, Action chaining.

    The DeleteAction does what it is supposed to ie delete the entity and the Retreival action continues to do its job of 'getting'.

    I think much of the problem lies in the tendency to put code other than flow control logic in the Action classes. IMO, as much as possible, Action classes should delegate any business operation to business classes. The Action class is a Control layer component. Its responsibilities are to direct the request to the appropriate classes that know how to do the actual job requested and perhaps perform preliminary setup so that the processing can be completed successfully. The job of deleting is not the DeleteAction's responsibility; rather the DeleteAction should know which class does the actual delete and delegate to it. Same thing for the RetrievalAction.

    My $0.02
    [ January 28, 2005: Message edited by: Junilu Lacar ]
    Karthik Guru
    Ranch Hand

    Joined: Mar 06, 2001
    Posts: 1209
    Hi Jason,

    Please correct me if I'm wrong.

    LoanServices.deleteLoan(long loanId)
    ------------------------------------
    1. LoanDAO.delete(loanId)


    LoanServices.getLoans()
    ------------------------------------
    1.return LoanDAO.getLoanList()


    Now, if I do action chaining,i still dont see any issues in chaining my deleteAction and my retreivalAction.

    Tomorrow if i need to move to some other workflow/screen after am done with a delete, then doing step#2 does'nt seem to help.

    LoanServices.deleteLoan(long loanId)
    ------------------------------------
    1. LoanDAO.delete(loanId)
    2. return LoanDAO.getLoanList()

    Disadvantages:

    1.I end up calling my business facade twice (and hence 2 business tier trips).
    2.one business method does not satisfy a use case completely (ie delete a loan and display all the loans in another page)
    3.I'm not sure how seamlessly can action chains work together. I mean am still referring to the context information i might have to populate for the next action to execute.
    George Franciscus
    author
    Ranch Hand

    Joined: Jan 25, 2005
    Posts: 32
    As rule I try to avoid action chaining wherever possible. Chaining tends to �smell� like a responsibility leak. The business layer is responsible for providing atomic business transactions. Since an action chain will not demarcate a transaction (unless you introduce JTA in your actions), it starts a path down a very slippery slope. If two actions in the chain need to act as a transactional unit, then action chaining will not support a cohesive transaction. If the two actions in the chain do not need transactional control, then you wont have the problem *today*.

    However, the application will be best understood during the development cycle. During the maintenance cycle someone could inadvertently create a non-atomic transaction. The greater the size of the application and breadth of the chain web, the greater the risk. The consequence of a non-atomic transaction could be great to the business community and not generally discovered until it reaches production. Good design and programming involves trying to make it bullet-proof into the future. A lofty goal, I know, but whatever you can do is better than nothing at all.

    Some people use chaining to re-use a �display� action from multiple create/update/delete actions. IMO the chain web starts to get bigger, more complicated and starts to diminish the benefit of re-use.

    If two actions are chained, then that generally means that there is a business relationship between the two actions � and that�s the job of the business layer. I think its better to couple that in the business layer rather than the web layer. A good way to test this relationship is to ask yourself � �If I were to swap out the web interface for a command-line interface, would things still hold up�. Often the answer to this question leads to decision to not chain the actions.

    Having said that, I have seen action chaining used where the designer was trying to achieve a conditional dispatching effect. There may be some merit to that, but its not all that frequent.


    George Franciscus<br />george.franciscus@nexcel.ca<br /><a href="http://www.nexcel.ca" target="_blank" rel="nofollow">http://www.nexcel.ca</a><br /><a href="http://www.manning.com/franciscus" target="_blank" rel="nofollow">http://www.manning.com/franciscus</a>
    Srikanth Shenoy
    author
    Ranch Hand

    Joined: Jan 24, 2004
    Posts: 184
    Jason,

    I a simple world, I would agree with you on the following sequence achievable without Action chaining.

    1. Given a list of loans, user selects a loan for deletion.
    2. System deletes selected loan.
    3. System presents user with a new list of loans.

    Maybe I didnt express my thoughts well in my last posting. Remember, I mentioned about some extra stuff to be taken care when displaying the Loan List. Examples "extra stuff" are whether to show the delete button in the LoanList or not. Manager should see this delete button, whereas Clerk should not.

    It does not make sense to store this kind of information in the LoanInfo (Model) object because this is a display detail. I would presume the LoanInfo object came from the EJB tier and does not (and should not ) have a clue about this delete button thing. MNeither should the LoanServices care about this (LoanServices is just a delegate for the Session EJB, I guess)

    That leaves you with the second approach of checking whether delete button has to be shown to the JSP. If-Else Logic in JSPs... Equally bad.

    IMHO, The better solution for this scenario is in the middle. Make this decision on whether to show the delete button or not based on the J2EE roles of the current user in the Action class.

    Why? Because this kind of logic is not business logic. It is a "sort of" view logic, that should not percolate to the view (JSP).

    Hence I treat the display of plain-vanilla loan list + plus these extra presentation logic things to be centralized in the Action class and called it an atomic operation (due to lack of better term). Since I want these to happen together, the logical thing is to chain the Actions.

    Having said that, IMHO, Action chaining is not a silver bullet, but best fit for scenarios like above.

    Thanks,
    Srikanth
    --------------------

    Srikanth Shenoy
    Author
    Struts Survival Guide : Basics to Best Practices

    [Woops! I tried to respond to your message but hit edit instead of reply and overwrote your text. The perils of being a moderator. All better now. -JM]
    [ January 31, 2005: Message edited by: Jason Menard ]
    Jason Menard
    Sheriff

    Joined: Nov 09, 2000
    Posts: 6450
    Srikanth: Examples "extra stuff" are whether to show the delete button in the LoanList or not. Manager should see this delete button, whereas Clerk should not.

    It does not make sense to store this kind of information in the LoanInfo (Model) object because this is a display detail. I would presume the LoanInfo object came from the EJB tier and does not (and should not ) have a clue about this delete button thing.


    As I see it, this is still a business rule and should therefore be handled by some other class and not the Struts Action or JSP. In this case, the business rule is something along the line of "If the user's role is Manager, the user may delete loans, otherwise they may not."

    So once we recognize that it is a rule, the question becomes how should we manage the rule and how should these rules be translated to the view? I like the idea of a "permissions" object to convey these rules, in conjunction with a set of rules or permissions managers.

    To illustrate what I'm talking about, we could do something like this:







    The LoanPermissions are now available to your JSP. You can now either create a custom tag to read the LoanPermissions object and display the correct buttons accordingly, or just have your JSP access the object and check each permission before displaying the appropriate button.

    This is a pretty simplistic example, and I've assumed a custom security solution with a User object which contains information about the user including his role, but the idea should be clear enough to make fit your specific situation. If we choose to go this route, we've totally removed the business rules from the Control and View layers and kept them where they probably are better suited. Not only that, when a new requirement comes down that alters the business rule, having the rules encapsulated like this it should be easier to make the change.
    [ January 31, 2005: Message edited by: Jason Menard ]
    Jason Menard
    Sheriff

    Joined: Nov 09, 2000
    Posts: 6450
    Karthik Guru: 1.I end up calling my business facade twice (and hence 2 business tier trips).

    Bingo. Just like you want to minimize the number of http requests made when possible, you also try to minimize the number of trips to the business tier.

    Karthik Guru: 2.one business method does not satisfy a use case completely (ie delete a loan and display all the loans in another page)

    This is also something I like to strive for. Aside from the ability to almost completely switch out your view and control, which let's be honest rarely happens, your application is much more testable by having your facade methods resolve a use case as completely as possible. When you're working in development teams where you have different people handling different application tiers you can really appreciate this.

    What I mean is if I'm the one coding up the Action classes, proceeding as I've shown makes it easier for me to mock the facade and test in isolation. Unit testing separate components in isolation (me testing the Actions in isolation, other developers testing the facade methods isoltated from the DAOs, etc...) provides more confidence during integration as well as making it easier to track down and correct any issues that might arise. As developers and architects, we should be keeping ease of testing in mind when developing our components and application architecture.

    Karthik Guru: 3.I'm not sure how seamlessly can action chains work together. I mean am still referring to the context information i might have to populate for the next action to execute.

    I'm going to make another post which I think might relate to this.
    Jason Menard
    Sheriff

    Joined: Nov 09, 2000
    Posts: 6450
    Let me say a little bit more against Action chaining...

    I've already talked about one way to eliminate Action chaining, but that's not the only way. Let's think about the problem. What's the reason most people do Action chaining? It seems to me that the most common reasin is to provide re-usability. Typically there will be a series of steps that we perform where some of it's the same, but maybe one key part is different.

    To illustrate what I'm talking about, let's expand on the Loan example we've been discussing. We;ve already determined that one thing we can do is delete loans from the list. Just for the sake of illustration, let's say we can also take an action to change the status of a loan to "ACTIVE", "PAID", or "PENDING". We handle that through an UpdateLoanStatusAction. There's also probably an action just to view the list of loans, ViewLoanListAction. Here are outlines for our Actions:

    DeleteLoanAction
    ----------------
    1. Delete selected Loan
    2. Get new list of loans to display

    UpdateLoanStatusAction
    ----------------------
    1. Change status of selected loan
    2. Get new list of loans to display

    ViewLoanListAction
    ------------------
    1. Get new list of loans to display

    We can see that one step is repeated in each Action. This is where we often fall into the trap of Action chaining. Often people will write the ViewLoanListAction and chain the other Actions to that.

    If we stop to think about it, what we have is an algorithm where really only one step is different, but otherwise the alogorithm is the same. Pattern guys and gals should be looking at this and immediately thinking Template Method pattern.

    Grabbing my trusty copy of Head First Design Patterns, I see the following definition for Template Method Pattern:



    Now all we have to do is extend LoanListAction and implement the processAction() method for each of our three Actions.







    I left it up to the overridden method to determine the correct ActionForward in order to keep the subclasses pretty much the same as traditional Actions, but you could factor this out as well if you wanted.

    I am a huge fan of the Template Method pattern when working with Struts. You've seen me use services in my examples in order to encapsulate the business logic. For the sake of testability, my service classes always implement interfaces and I retrieve them via an implementation of ServiceFactory which is also an interface. In order to maximize testability, it's desirable to pass the implentation of ServiceFactory into the Action. So Ideally I would have an Action with an execute() method having the following signature:



    In order to pass in the ServiceFactory to such a method, each Action would first have to obtain a reference to the correct impplementation of ServiceFactory. Template Method pattern to the rescue. Here's my algorithm for each Action:

    1. Lookup and obtain a reference to ServiceFactory
    2. process remaining ActionLogic, allowing logic access to ServiceFactory implementation

    Using Template Method pattern I can do the following:



    My implementation might look like this:



    All my Actions have to do is extend ServiceFactoryAction and implement the abstract execute() method with the ServiceFactory argument. When I'm unit testing, I can easily test the overridden method passing in a mock ServiceFactory which returns mock services in order to test in isolation.

    I've been using this pattern with Struts for awhile now, and from my experience, the Template Method pattern is a great tool to have in your toolbox when it comes to building complex Struts applications.
    [ January 31, 2005: Message edited by: Jason Menard ]
     
    It is sorta covered in the JavaRanch Style Guide.
     
    subject: [Struts Recipes] Action chaining: recipe for disaster?