Return-Path: Delivered-To: apmail-myfaces-dev-archive@www.apache.org Received: (qmail 6973 invoked from network); 13 Aug 2007 17:02:23 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 13 Aug 2007 17:02:23 -0000 Received: (qmail 46527 invoked by uid 500); 13 Aug 2007 17:02:20 -0000 Delivered-To: apmail-myfaces-dev-archive@myfaces.apache.org Received: (qmail 46462 invoked by uid 500); 13 Aug 2007 17:02:20 -0000 Mailing-List: contact dev-help@myfaces.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "MyFaces Development" Delivered-To: mailing list dev@myfaces.apache.org Received: (qmail 46451 invoked by uid 99); 13 Aug 2007 17:02:20 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 13 Aug 2007 10:02:20 -0700 X-ASF-Spam-Status: No, hits=2.0 required=10.0 tests=HTML_MESSAGE,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of simon.lessard.3@gmail.com designates 209.85.132.246 as permitted sender) Received: from [209.85.132.246] (HELO an-out-0708.google.com) (209.85.132.246) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 13 Aug 2007 17:02:12 +0000 Received: by an-out-0708.google.com with SMTP id c3so326133ana for ; Mon, 13 Aug 2007 10:01:51 -0700 (PDT) DKIM-Signature: a=rsa-sha1; c=relaxed/relaxed; d=gmail.com; s=beta; h=domainkey-signature:received:received:message-id:date:from:to:subject:in-reply-to:mime-version:content-type:references; b=f+xLzFsw5FCycqhCZSPtQ/HtG5hy/4lUGUB93L9sV2o9keqhIkpgEcXbe4u5AnFSTxb9rECczDuOybk5+ljHa8BjpVmk9UuXQOtYZ0hfRK+jeuBe2NloFn1JOxNR3mZhwUSG7L+ikPw7t/UWtH31DronfyuRsPxUVi8auFCMMQE= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:to:subject:in-reply-to:mime-version:content-type:references; b=H4jvbcciymaiaguCakyp6NujAIFb1Fdddaki/H2rE71/wfM3BAboaTbNOJhGl6GIe9DcOjhAPdEgmRkuvOil6yi1fVboTSTbas9i4JmyGUQ+gtqVnTN6K06yQEv4r6UlAFp8HoFGjYIjrOSELZmfOaftYOrQa1ZMAFbCFd1DCs4= Received: by 10.100.13.12 with SMTP id 12mr573724anm.1187024508865; Mon, 13 Aug 2007 10:01:48 -0700 (PDT) Received: by 10.100.135.2 with HTTP; Mon, 13 Aug 2007 10:01:45 -0700 (PDT) Message-ID: <254acf980708131001p79668e4fu5e6ad44e7804978@mail.gmail.com> Date: Mon, 13 Aug 2007 13:01:45 -0400 From: "Simon Lessard" To: "MyFaces Development" Subject: Re: [TRINIDAD] ProcessMenuModel changes In-Reply-To: <254acf980708130959v471f2f3br1886a61e6a884e38@mail.gmail.com> MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="----=_Part_62358_10363529.1187024505653" References: <254acf980708101010v5132f336k95fa2844177706ef@mail.gmail.com> <254acf980708130620g45611a50i6b77362c34be6e4d@mail.gmail.com> <5bbf92e70708130628ld3807a9gafb9c4c1e1acbf25@mail.gmail.com> <5a99335f0708130629g45e0361fs9958065ca08add6@mail.gmail.com> <6dac79b90708130942w4f94710fla76f72ed0d2917f9@mail.gmail.com> <6dac79b90708130943w1b2cc657q463a549f0b386ebc@mail.gmail.com> <254acf980708130959v471f2f3br1886a61e6a884e38@mail.gmail.com> X-Virus-Checked: Checked by ClamAV on apache.org ------=_Part_62358_10363529.1187024505653 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline The real issue imho is more with the methods linked with previous/next step management. I guess those could be easily placed in the subclass though as the train itself won't use them. - public abstract boolean isNextStepAvailable(); /* Could be optional or maybe in a subclass, this would check if there's a step before the current one */ - public abstract boolean isPreviousStepAvailable(); /* As above */ - public abstract Object getNextStep(); /* As above */ - public abstract Object getPreviousStep(); /* As above */ Regards, ~ Simon On 8/13/07, Simon Lessard wrote: > > Hmmm, isn't that what I suggested? Current class is ProcessMenuModel, the > new one does not include the "Menu" part. > > On 8/13/07, Adam Winer wrote: > > > > So... what if we left the current class alone, in terms of its API and > > name, and just exposed a new base class? > > > > -- Adam > > > > > > On 8/13/07, Adam Winer wrote: > > > I've got some concern for backwards compatibility - this'll break > > > some code on our end. I'm hoping Jeanne will have some comments > > > too. > > > > > > -- Adam > > > > > > > > > On 8/13/07, Martin Marinschek wrote: > > > > No voice means you can go ahead ;) > > > > > > > > regards, > > > > > > > > Martin > > > > > > > > On 8/13/07, Danny Robinson < dannyjrobinson@gmail.com> wrote: > > > > > Sorry Simon, I have little/no experience with this part of > > Trinidad so can't > > > > > comment. I trust your judgement, so you have my vote if you need > > it ;-) > > > > > > > > > > > > > > > On 8/13/07, Simon Lessard wrote: > > > > > > So I assume it would be +0 for everyone? > > > > > > > > > > > > > > > > > > > > > > > > On 8/10/07, Simon Lessard < simon.lessard.3@gmail.com> wrote: > > > > > > > Hello everybody, > > > > > > > > > > > > > > Currently Trinidad includes a ProcessMenuModel class that > > contains > > > > > undesirable methods. The complete method list (not including > > inherited ones) > > > > > is: > > > > > > > > > > > > > > > > > > > > > public boolean isImmediate() > > > > > > > public boolean isReadOnly() > > > > > > > public boolean isVisited() > > > > > > > public void clearMaxPath() > > > > > > > public void setMaxPathKey(Object maxPathKey) > > > > > > > public Object getMaxPathKey()The methods involving maxPathKey > > are the > > > > > ones annoying me the most. However, as it's part of the public API > > we have > > > > > to keep backward compatibility as much as possible. Note also that > > > > > isReadOnly should not named that way as readOnly support was > > removed from > > > > > process classes in favor of disabled since a readOnly link did not > > make much > > > > > sense. > > > > > > > > > > > > > > Anyway, I would rather have the following class structure and > > method > > > > > signatures: > > > > > > > > > > > > > > public abstract class ProcessModel > > > > > > > > > > > > > > > > > > > > > public abstract boolean isDisabled(); > > > > > > > public abstract boolean isImmediate(); > > > > > > > public abstract boolean isVisited(); > > > > > > > public abstract boolean isNextStepAvailable(); /* Could be > > optional or > > > > > maybe in a subclass, this would check if there's a step before the > > current > > > > > one */ > > > > > > > public abstract boolean isPreviousStepAvailable(); /* As above > > */ > > > > > > > public abstract Object getNextStep(); /* As above */ > > > > > > > public abstract Object getPreviousStep(); /* As above */public > > class > > > > > MaxPathKeyProcessModel extends ProcessModel /* Or a better name */ > > > > > > > > > > > > > > > > > > > > > Implements all methods using the old ProcessMenuModel > > code.@Deprecated > > > > > > > public class ProcessMenuModel extends MaxPathKeyProcessModel > > > > > > > > > > > > > > > > > > > > > Empty class except for isReadOnly() that should return > > > > > super.isDisabled() > > > > > > > The structure above would clean up the Model class that really > > shouldn't > > > > > contain very implementation specific code like the max path key > > algorithm > > > > > and would allow us to add new ProcessModel classes with more > > > > > functionalities. For example I had one using a mode for step > > access right > > > > > like: MAX_PLUS_NEXT, MAX, ANY, NEXT_ONLY, etc. > > > > > > > > > > > > > > The previous/next step methods could be useful for page > > templates since > > > > > it would be possible to include the train in the header as well as > > a > > > > > previous and next step buttons in the page footer in a generic way > > using the > > > > > very same process model. Note that we might have to also include > > methods > > > > > like isPreviousVisited(), isPreviousDisabled() and such to fully > > support > > > > > that. > > > > > > > > > > > > > > Opinions, suggestions? > > > > > > > > > > > > > > > > > > > > > Regards, > > > > > > > > > > > > > > ~ Simon > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > Chordiant Software Inc. > > > > > www.chordiant.com > > > > > > > > > > > > -- > > > > > > > > http://www.irian.at > > > > > > > > Your JSF powerhouse - > > > > JSF Consulting, Development and > > > > Courses in English and German > > > > > > > > Professional Support for Apache MyFaces > > > > > > > > > > > ------=_Part_62358_10363529.1187024505653 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline The real issue imho is more with the methods linked with previous/next step management.  I guess those could be easily placed in the subclass though as the train itself won't use them.
  • public abstract boolean isNextStepAvailable(); /* Could be optional or maybe in a subclass, this would check if there's a step before the current one */
  • public abstract boolean isPreviousStepAvailable(); /* As above */
  • public abstract Object getNextStep(); /* As above */
  • public abstract Object getPreviousStep(); /* As above */

Regards,

~ Simon

On 8/13/07, Simon Lessard <simon.lessard.3@gmail.com> wrote:
Hmmm, isn't that what I suggested? Current class is ProcessMenuModel, the new one does not include the "Menu" part.


On 8/13/07, Adam Winer <awiner@gmail.com> wrote:
So...  what if we left the current class alone, in terms of its API and
name, and just exposed a new base class?

-- Adam


On 8/13/07, Adam Winer <awiner@gmail.com > wrote:
> I've got some concern for backwards compatibility - this'll break
> some code on our end.  I'm hoping Jeanne will have some comments
> too.
>
> -- Adam
>
>
> On 8/13/07, Martin Marinschek < martin.marinschek@gmail.com > wrote:
> > No voice means you can go ahead ;)
> >
> > regards,
> >
> > Martin
> >
> > On 8/13/07, Danny Robinson < dannyjrobinson@gmail.com> wrote:
> > > Sorry Simon, I have little/no experience with this part of Trinidad so can't
> > > comment.  I trust your judgement, so you have my vote if you need it ;-)
> > >
> > >
> > > On 8/13/07, Simon Lessard <simon.lessard.3@gmail.com > wrote:
> > > > So I assume it would be +0 for everyone?
> > > >
> > > >
> > > >
> > > > On 8/10/07, Simon Lessard < simon.lessard.3@gmail.com> wrote:
> > > > > Hello everybody,
> > > > >
> > > > > Currently Trinidad includes a ProcessMenuModel class that contains
> > > undesirable methods. The complete method list (not including inherited ones)
> > > is:
> > > > >
> > > > >
> > > > > public boolean isImmediate()
> > > > > public boolean isReadOnly()
> > > > > public boolean isVisited()
> > > > > public void clearMaxPath()
> > > > > public void setMaxPathKey(Object maxPathKey)
> > > > > public Object getMaxPathKey()The methods involving maxPathKey are the
> > > ones annoying me the most. However, as it's part of the public API we have
> > > to keep backward compatibility as much as possible. Note also that
> > > isReadOnly should not named that way as readOnly support was removed from
> > > process classes in favor of disabled since a readOnly link did not make much
> > > sense.
> > > > >
> > > > > Anyway, I would rather have the following class structure and method
> > > signatures:
> > > > >
> > > > > public abstract class ProcessModel
> > > > >
> > > > >
> > > > > public abstract boolean isDisabled();
> > > > > public abstract boolean isImmediate();
> > > > > public abstract boolean isVisited();
> > > > > public abstract boolean isNextStepAvailable(); /* Could be optional or
> > > maybe in a subclass, this would check if there's a step before the current
> > > one */
> > > > > public abstract boolean isPreviousStepAvailable(); /* As above */
> > > > > public abstract Object getNextStep(); /* As above */
> > > > > public abstract Object getPreviousStep(); /* As above */public class
> > > MaxPathKeyProcessModel extends ProcessModel /* Or a better name */
> > > > >
> > > > >
> > > > > Implements all methods using the old ProcessMenuModel code.@Deprecated
> > > > > public class ProcessMenuModel extends MaxPathKeyProcessModel
> > > > >
> > > > >
> > > > > Empty class except for isReadOnly() that should return
> > > super.isDisabled()
> > > > > The structure above would clean up the Model class that really shouldn't
> > > contain very implementation specific code like the max path key algorithm
> > > and would allow us to add new ProcessModel classes with more
> > > functionalities. For example I had one using a mode for step access right
> > > like: MAX_PLUS_NEXT, MAX, ANY, NEXT_ONLY, etc.
> > > > >
> > > > > The previous/next step methods could be useful for page templates since
> > > it would be possible to include the train in the header as well as a
> > > previous and next step buttons in the page footer in a generic way using the
> > > very same process model. Note that we might have to also include methods
> > > like isPreviousVisited(), isPreviousDisabled() and such to fully support
> > > that.
> > > > >
> > > > > Opinions, suggestions?
> > > > >
> > > > >
> > > > > Regards,
> > > > >
> > > > > ~ Simon
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > >
> > >
> > >
> > > --
> > > Chordiant Software Inc.
> > > www.chordiant.com
> >
> >
> > --
> >
> > http://www.irian.at
> >
> > Your JSF powerhouse -
> > JSF Consulting, Development and
> > Courses in English and German
> >
> > Professional Support for Apache MyFaces
> >
>


------=_Part_62358_10363529.1187024505653--