On 9/10/06, Rahul Akolkar <[EMAIL PROTECTED]> wrote:
On 9/9/06, Craig McClanahan <[EMAIL PROTECTED]> wrote:
> Sean's changes fo the dialog2 sandbox implementation in r440205 includes
the
> following change that I object to:
>
> ---
shale/sandbox/shale-dialog2/src/main/java/org/apache/shale/dialog2/faces/Dialog2NavigationHandler.java
> (original)
> +++
>
shale/sandbox/shale-dialog2/src/main/java/org/apache/shale/dialog2/faces/Dialog2NavigationHandler.java
> Mon Sep 4 16:52:55 2006
> @@ -135,6 +135,9 @@
> + context + "' with navigation to viewId '"
> + viewId + "'");
> }
> + if (viewId == null) {
> + original.handleNavigation(context, fromAction,
outcome);
> + }
> }
> navigate(context, viewId);
>
>
> My objection is on two grounds:
>
> * Philosophically, navigation within a dialog should be self contained,
> and attempts to navigate via an outcome not defined by the dialog
> definition should be treated as an error condition. Otherwise, we're
> going to find ourselves in situations when the actual navigation being
> performed is non-deterministic ... it's based on what (if any) dialogs
> called me as a subdialog, so I have to do a lot more searching to
> find problems. It also makes static analysis of an application to
> determine
> whether all its outcome values are legal virtually impossible.
>
<snip/>
I'm in agreement with the above, and we cannot undermine the benefits
of static analysis -- we need that end-to-end approach from modeling
to generation to testing.
As a sidebar, there are practical considerations (such as global
navbars) that we should perhaps explore via sample dialogs in the
sandbox, and attempt to establish a set of best practice guidelines
for the same (such as mapping those to some end states?).
I've got a big chunk of time reserved this afternoon to focuson dialog2, and
examples are definitely on the list of things I want to work on. Besides
menus, another really common usecase I'd like to see us cover gracefully is
a popup window (which starts its own dialog instance, per our current
approach) that can gracefuly "pull" data from the dialog of the main page at
startup, and "push" data back again when the popup dialog finishes.
Although this particular change only delegates to the original
> navigation handler if the last state executed returned null, the
> stated justification for this action ("delegate when we don't know
> what the response means") does not match the reality here. In
> particular, a null return from a state is a *deliberate* decision by
> the dialog to stay on the current view id. That decision should be
> respected.
>
<snap/>
I've always been seeing that change in light of Sean's earlier commit
r440216 [1]. This means that the legacy implementation returns null in
two cases, deliberately or on IllegalStateException in transition()
(which is masked into a null return).
The latter sounds like the wrong approach -- to be consistent with a "self
contained" paradigm for dialog definitions, it should really throw the
exception. Among the details to be addressed is documentation, and a
tightening up on the Javadoc contracts about exceptions and return values.
* Technically, this particular change doesn't look like it will accomplish
> anything anyway, because it is not followed by a "return" statement.
> Instead, control will fall through to the call to navigate() anyway.
>
<snip/>
Indeed.
-Rahul
[1] http://svn.apache.org/viewvc?view=rev&revision=440216
Craig
