struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Cooper <mart...@apache.org>
Subject RE: Another bright idea, make "indexed" work with JSTL forEach and friends
Date Sun, 05 Jan 2003 01:23:17 GMT


On Sat, 4 Jan 2003, James Turner wrote:

> Ok, here's a more practical reason to do it in the base release...
>
> The Struts-EL package doesn't handle the indexed tag at all, it relies
> on the fact that it extends the org.apache.struts.taglib.html versions
> of the tags which in turn eventually extend BaseHandlerTag.  So, to
> implement it in Struts-EL, I'll have to more prepareIndex all the way up
> into the Struts-EL class (duplicated the code or wrapping it), and
> change all the Struts-EL classes to extend that class instead.  This
> means implementing several dozen new classes in Struts-EL to avoid
> extending one method in the base Struts release.

Sigh. OK, OK.

But three changes I'd like to see in the code you posted earlier:

1) Instead of calling Class.forName(), you should use
RequestUtils.applicationClass(), to make sure the context class loader is
tried first.

2) Empty catch clauses are evil. ;-) You should at least log a debug
message so that real problems can be debugged more easily.

3) Always use braces with if clauses. I know the code has plenty of cases
where that isn't done, but it's good practice to do that. I seem to recall
Craig admitting that he should have done that in the original code base.
;-)

--
Martin Cooper


>
> James
>
> > -----Original Message-----
> > From: Martin Cooper [mailto:martinc@apache.org]
> > Sent: Saturday, January 04, 2003 4:37 PM
> > To: Struts Developers List
> > Subject: RE: Another bright idea, make "indexed" work with
> > JSTL forEach and friends
> >
> >
> >
> >
> > On Sat, 4 Jan 2003, James Turner wrote:
> >
> > > On Sat, 4 Jan 2003, Martin Cooper wrote:
> > >
> > > > If you want to do this, I'd rather see it happen in the
> > html-el taglib
> > > > than the regular html taglib. Struts-EL already depends on
> > > > JSTL, and is
> > > > designed to work in cooperation with it, so it's a much more
> > > > natural fit
> > > > than trying to sneak JSTL functionality into the regular tags.
> > >
> > > I mildly disagree.  EL is to allow struts HTML tags to use
> > EL syntax.
> >
> > Yes. And why would you want to do that? Because you're
> > already using JSTL
> > tags in your pages, and you want the two to work together.
> >
> > > This deals with letting the standard tags find JSTL looping
> > constructs.
> >
> > Yes. And why would you want to do that? Because you're
> > already using JSTL
> > tags in your pages, and you want the two to work together.
> >
> > Notice the remarkable similarity in the two reasons for using
> > these pieces
> > of functionality? ;-)
> >
> > That's why I think your suggestion fits much better in
> > Struts-EL than in
> > the Struts core. Once we require Servlets 2.3 / JSP 1.2 for
> > Struts, then
> > I'm all for having this in the core, along with the rest of Struts-EL.
> > Prior to that, I just don't like the idea of muddying the
> > core html taglib
> > with references to JSTL, especially when you have to do it all through
> > reflection.
> >
> > --
> > Martin Cooper
> >
> >
> > > As is, you can "*almost* entirely eliminate all the Struts
> > tags except
> > > for the HTML tags in favor of JSTL substitutes, since only
> > the HTML tags
> > > deal with things like actions.  By implementing this, we
> > can eliminate
> > > having to use the logic taglibs at all.  And the change is
> > pretty darn
> > > innocuous, here's the revisted code from BaseHandlerTag, which works
> > > very nicely.  Note that I'm not even referencing org.apache
> > stuff.  And
> > > the JSTL stuff is only ever invoked if it fails to find the
> > Iterate tag.
> > >
> > > One thing I'm considering is caching the classes and
> > methods so that the
> > > reflection doesn't need to happen on every invokation.
> > >
> > >     protected void prepareIndex(StringBuffer handlers, String name)
> > > throws JspException {
> > > 	int index = 0;
> > > 	boolean found = false;
> > >
> > >         // look for outer iterate tag
> > >         IterateTag iterateTag = (IterateTag)
> > findAncestorWithClass(this,
> > > IterateTag.class);
> > > 	// Look for JSTL loops
> > > 	if (iterateTag == null) {
> > > 	    try {
> > > 		Class loopClass =
> > > Class.forName("javax.servlet.jsp.jstl.core.LoopTagSupport");
> > > 		Object loopTag = findAncestorWithClass(this, loopClass);
> > > 		if (loopTag != null) {
> > > 		    Method getStatus =
> > > 			loopClass.getDeclaredMethod("getLoopStatus",
> > > null);
> > > 		    Object status = getStatus.invoke(loopTag, null);
> > > 		    Class statusClass =
> > >
> > > Class.forName("javax.servlet.jsp.jstl.core.LoopTagStatus");
> > > 		    Method getIndex =
> > > 			statusClass.getDeclaredMethod("getIndex", null);
> > > 		    Integer ind = (Integer) getIndex.invoke(status,
> > > null);
> > > 		    index = ind.intValue();
> > > 		    found = true;
> > > 		}
> > > 	    }
> > > 	    catch (ClassNotFoundException ex) {}
> > > 	    catch (NoSuchMethodException ex) {}
> > > 	    catch (IllegalAccessException ex) {}
> > > 	    catch (IllegalArgumentException ex) {}
> > > 	    catch (InvocationTargetException ex) {}
> > > 	    catch (NullPointerException ex) {}
> > > 	    catch (ExceptionInInitializerError ex) {}
> > > 	} else {
> > > 	    index = iterateTag.getIndex();
> > > 	    found = true;
> > > 	}
> > >         if (!found) {
> > >             // this tag should only be nested in iteratetag, if it's
> > > not, throw exception
> > >             JspException e = new
> > > JspException(messages.getMessage("indexed.noEnclosingIterate"));
> > >             RequestUtils.saveException(pageContext, e);
> > >             throw e;
> > >         }
> > >         if (name != null)
> > >             handlers.append(name);
> > >         handlers.append("[");
> > >         handlers.append(index);
> > >         handlers.append("]");
> > >         if (name != null)
> > >             handlers.append(".");
> > >     }
> > >
> > >
> > >
> > > --
> > > To unsubscribe, e-mail:
> > <mailto:struts-dev-> unsubscribe@jakarta.apache.org>
> > > For
> > additional commands,
> > e-mail: <mailto:struts-dev-help@jakarta.apache.org>
> > >
> > >
> >
> >
> > --
> > To unsubscribe, e-mail:
> > <mailto:struts-dev-> unsubscribe@jakarta.apache.org>
> > For
> > additional commands,
> > e-mail: <mailto:struts-dev-help@jakarta.apache.org>
> >
>
>
>
> --
> To unsubscribe, e-mail:   <mailto:struts-dev-unsubscribe@jakarta.apache.org>
> For additional commands, e-mail: <mailto:struts-dev-help@jakarta.apache.org>
>
>


--
To unsubscribe, e-mail:   <mailto:struts-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:struts-dev-help@jakarta.apache.org>


Mime
View raw message