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:[EMAIL PROTECTED]] > > 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-> [EMAIL PROTECTED]> > > > For > > additional commands, > > e-mail: <mailto:[EMAIL PROTECTED]> > > > > > > > > > > > > -- > > To unsubscribe, e-mail: > > <mailto:struts-dev-> [EMAIL PROTECTED]> > > For > > additional commands, > > e-mail: <mailto:[EMAIL PROTECTED]> > > > > > > -- > To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> > For additional commands, e-mail: <mailto:[EMAIL PROTECTED]> > > -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
