| Summary: | Minor performance enhancement in set result evaluation | ||
|---|---|---|---|
| Product: | Taglibs | Reporter: | Jeremy Boynes <jboynes> |
| Component: | Standard Taglib | Assignee: | Tomcat Developers Mailing List <dev> |
| Status: | RESOLVED FIXED | ||
| Severity: | enhancement | ||
| Priority: | P2 | ||
| Version: | 1.2.0 | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | All | ||
| Attachments: | patch to remove one check and a unneeded call to BodyContent#getString | ||
|
Description
Jeremy Boynes
2010-07-03 13:26:05 UTC
Created attachment 25692 [details]
patch to remove one check and a unneeded call to BodyContent#getString
There's a functionality change here: If valueSpecified is false, yet there is a value, it used to return value. Now it returns bodyContent based results. I don't see this being in the spec, so presumably this is bad design that we're stuck with supporting; a protected status variable in addition to the main one, rather than a 'isValueSpecified()' method. Thoughts? value and valueSpecified are both protected variables and part of our implementation rather than the spec. The container will just call setValue(Object) to set the attribute's value and both our subclasses set both variables in their implementation. My thought was that the contract this has with its subclasses is that both must be set and if that is met the change works. The original tests I added weren't doing that which was why it was OK to change them. Agreed that the contract would be that both must be set. I've added javadoc that the two attributes must be in sync as a part of this commit: svn ci -m "Improving the performance of SetSupport by only calling bodyContent.getString() once per Jeremy's patch #49547"Sending impl/src/main/java/org/apache/taglibs/standard/tag/common/core/SetSupport.java Sending impl/src/test/java/org/apache/taglibs/standard/tag/common/core/TestSetSupport.java Transmitting file data .. Committed revision 960259. |