well, the real information for the BreadcrumbsTrail is currently in a
(breadCrumbTrail) String attribute in the HttpSession.
Isn't wrapping this string in a Serializable Breadcrumb class a bit overdone
?
It will increase the Session size, and will not boost
performance/scalability either.

So, I like to keep it in BreadcrumbsTag for now if you don't mind.

regards,
Harry

2009/5/11 Andrew Jaquith <[email protected]>

> PS. This breadcrumb code is absolutely NOT meant to sit in the
> AbstractPageActionBean class. It should really be put somewhere else.
> Actually, I think what we should really do is create a Breadcrumbs
> class and have it made available as a property of WikiSession.
>
> On Mon, May 11, 2009 at 1:43 PM, Harry Metske <[email protected]>
> wrote:
> > yup, I'll fix that tomorrow too.
> >
> > I will also switch to the commit-then-review approach from now on.
> >
> > regards,
> > Harry
> >
> >
> >
> > 2009/5/10 <[email protected]>
> >
> >> Author: jalkanen
> >> Date: Sun May 10 18:14:07 2009
> >> New Revision: 773375
> >>
> >> URL: http://svn.apache.org/viewvc?rev=773375&view=rev
> >> Log:
> >> Added a FIXME
> >>
> >> Modified:
> >>
> >>
>  
> incubator/jspwiki/trunk/src/java/org/apache/wiki/action/AbstractPageActionBean.java
> >>
> >> Modified:
> >>
> incubator/jspwiki/trunk/src/java/org/apache/wiki/action/AbstractPageActionBean.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/incubator/jspwiki/trunk/src/java/org/apache/wiki/action/AbstractPageActionBean.java?rev=773375&r1=773374&r2=773375&view=diff
> >>
> >>
> ==============================================================================
> >> ---
> >>
> incubator/jspwiki/trunk/src/java/org/apache/wiki/action/AbstractPageActionBean.java
> >> (original)
> >> +++
> >>
> incubator/jspwiki/trunk/src/java/org/apache/wiki/action/AbstractPageActionBean.java
> >> Sun May 10 18:14:07 2009
> >> @@ -74,6 +74,7 @@
> >>      *
> >>      * @param pageName the pageName to be removed from the breadcrumb
> >>      */
> >> +    // FIXME: Is this in the right place? Shouldn't this be a static
> >> method in BreadcrumbsTag?
> >>     void deleteFromBreadCrumb( String pageName )
> >>     {
> >>         HttpSession session = getContext().getRequest().getSession(
> false
> >> );
> >>
> >>
> >>
> >
>

Reply via email to