On Nov 26, 2007 4:57 PM, Geoff Callender
<[EMAIL PROTECTED]> wrote:
> Hi,
>
> I'm looking for suggestions on how to improve this EditPerson page to
> T5 best practice.
Hello Geoff. See comments below, though i can't make any claims to
best-practice myself :)
> private Long _personId;
>
> @Persist
> private Person _person;
>
> public void onActivate(Long id) { _personId = id; }
>
> public Long onPassivate() { return _personId; }
>
> void onPrepareFromForm() {
> if (_person == null || _person.getId().equals(_personId)) {
> _person = getSecurityFinderService().findPerson(_personId);
> }
> }
>
> Object onSuccess() {
> if (...a bit of validation logic detects an error...) {
> _form.recordError(...);
> return null;
> }
> etc...
> _person = null;
> return _nextPage;
> }
>
> Object onActionFromCancel() {
> _person = null;
> return _previousPage;
> }
>
> void cleanupRender() {
> _form.clearErrors();
> }
>
> Some notes and questions:
>
> 1. Persist - Is there an alternative? It's used here to keep _person
> populated in case onSuccess() detects an error and redisplays the page.
Well, you could always look up the person again from the
SecurityFinderService, but i suppose you don't want to pay for the
lookup again... In that case, i don't know of an alternative.
Note that you probably want to use flash (perhaps even client?) persistence.
The form component does accept a 'context' parameter which might allow
you to eliminate onActivate/onPassivate in favour of parameters to
onPrepareFromForm() and a getter for the id. I haven't played with
that much so if you do pursue it, please report back :)
And as a side note, shouldn't the second condition in
onPrepareFromForm() be negated? You want to lookup the person if the
ids _don't_ match, right?
> 2. Housekeeping - I'm nullifying _person in each method that returns a
> different page. This seems clumsy. Suggestions?
Why? Tapestry will take care of basic housekeeping. If you're
nulling it out to force a subsequent request to refresh the person's
state, then a better alternative is to use flash persistence (or no
persistence).
On a similiar note, if you set the default persistence strategy of the
form to flash (via a metadata annotation), it will apply this to its
validation tracker as well. That would allow you to eliminate
cleanupRender().
> 3. Thread-safety - is there a thread-safety issue if the user has
> opened a new window in same browser - can T5 tell them apart or do
> they share the same HttpSession?
Yes, they will share the same session, so anything you @Persist could
be a problem (unless its persisted on the client). I don't know of
anything tap5 does to help with this, so hopefully someone more
knowledgeable will chime in.
Again, i'd reconsider just looking up the person again.
> 4. onPrepareFromForm() - this looks a bit clumsy to me. If the user
> hits Reload, or if the user hits the Back button then "pagelinks" back
> in, onPrepareFromForm() wlll not call findPerson(...). The _person
> isn't refreshed at all! Thoughts?
Using flash persistence should solve both those issues, yeah?
Ok, so it seems most of my comments boil down to: don't persist, and
use flash/client strategies if you must :)
Let me throw something else in to break the monotony: you can
eliminate the conditional in onSuccess() by defining an onValidate()
handler. If onValidate() records errors, tapestry will know not to
call onSuccess().
Ok, hope that helps some.
Cheers, lasitha.
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
