Michael Jouravlev wrote the following on 7/6/2005 3:22 AM:
Can you explain why is it so bad, besides that it is "bad design" and
"defeats pupose of what ActionForms should be used for"? What
ActionForms should be used for, anyway? It is just a class, which has
a simple lifecycle, maintained by Struts. Why the religious fear of
not doing something against how it "should be used for"? If it works,
why not?
Obviously different people will have different ideas for how they should
do things and there is something to be said for "hey if it works why
not," but here are some things to consider...
1) The person asking I believe was a beginner to Struts. Suggesting to
have a DAO used in ActionForm is something definitely unorthodox and
since the person is new to the framework it is unlikely he'll see many
examples of this and it can get confusing if he's seeing things done in
some non-orthodox way. (Granted 'orthodox' doesn't always equate to
best-practice, but I'll get into why I think it's bad in some other points).
2) Not only is more difficult for a beginner to start doing things in an
unorthodox way, it also will be more difficult for someone else that
would have to inherit the code to understand. Poll all the gurus that
have been doing Struts for many years and ask them if they'd ever expect
to see database calls from within an ActionForm.
Now, putting the above aside, here are some reasons why it's poor design...
3) You asked what the purpose of the ActionForm was for and first off I
must state that I don't like the tie in of ActionForms with Struts. I
like the approach JSP takes much better, but since I'm working with
Struts here the purpose as I understand it is mainly a) To have a tie in
to the html:form tag for pulling out properties using the html tag and
b) for providing a mechanism to ensure the user's input remains for when
validation fails and you are returned back to the page. There has been
debate on this list as to where exactly the ActionForm fits in the
architecture, but rarely have I seen it debated that it should serve up
business logic making calls to the backend. The typical design rule is
usually separation of responsibilities. If you now start having
ActionForms deal with making calls to your persistence layer, you know
just added one more layer to have to maintain this code. Granted, you
could make things 'sort of' clean by making sure to call some delegate
or facade that hides the backend call, which I like to do anyway in my
Action classes. I think it's a good design practice to try to limit
where types of calls are being made. By moving backend calls to the
ActionForm you've added a whole other layer to have to maintain those
calls (other than say just Actions).
4) Since the ActionForm is used to capture user inputted data, it seems
against the grain to have the ActionForm itself populating a state that
the user wants to edit. Also, typically ActionForms aren't just used to
capture 'edit' data but also data for a fresh "add" operation. So now,
not only have you added backend calls from your ActionForm, but you now
added more complex logic to have to decide when to make those calls and
when not to (is the form being used for an edit operation or for a fresh
add operation?) Actually I'm not even sure how you would accomplish
this? You tell me - how are you going to know it's an 'add or edit'
operation while in the reset method? You aren't going to want to
populate all the employee/user info if it's an 'add', yet somehow you'll
have to know this? I guess you'll have to use request.getParameter
because you won't be able to use the form fields since they don't get
populate until 'after' the reset is called. So you see the kind of mess
things become? You have to tell the new person.. "Oh wait, don't use the
form field here to determine if it's an edit or add, because that
property hasn't been populated yet."
5) Another problem is consistency. Most users of Struts understand that
they submit a form and it goes to an Action and then you forward
somewhere. Pretty basic. So imagine the person that inherits the code
where the edit information is done by the ActionForm itself... the
developer sees himself click the "edit this employee" button and a form
page comes up populated with the employees info to edit. He realizes
something is a bit wrong and he needs to debug the app. More than likely
he's going to look first at the Action the form submits to since that's
what most of the forms do... but low and behold, he/she doesn't see
where the population of the form is taking place since it's being done
in the ActionForm itself.
6) Why not have all your forms simply submit to the generic
ForwardAction and you can do all the logic in the reset, not just
pre-population? You can pull out request parameters and then do what you
need to do there... call updates, inserts, whatever. Actually I'd
recommend doing this over doing some of this in resets and some of it in
Actions. At least it would be consistent. Or, if you want, do the
pre-population in your own RequestProcessor (kidding of course, but the
point is you could do things in many places, but I wouldn't say it's the
best practice if it doesn't gain you anything versus doing it in a
standard consistent way. You save nothing doing prepopulation in a reset
method versus an Action so I'd argue to be consistent with how people
expect to use the framework - especially when giving advice to a newbie.
Sure if you've been using Struts for a long time and feel like messing
around, go for it, but for someone new to the framework I'm sure they'd
appreciate a consistent simple approach.
Anyway, I'm sure there is more I can think of but it's 6:18am and I
haven't been to sleep yet. (So excuse the many grammar mistakes in the
above also.) Work will be so much fun tomorrow:)
--
Rick
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
