jakarta-taglibs-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kris Schneider <k...@dotech.com>
Subject RE: Review request WAS: escapeXml() optimizations
Date Wed, 11 Feb 2004 12:57:09 GMT
Just to clarify, it's not just the proposed new version that doesn't check for
input == null, the current version fails to do that as well. I dumped my
feedback into bugzilla, I didn't realize a formal bug had been opened. I'm used
to seeing the bugzilla stuff auto-posted to the dev lists, but it looks like
taglibs-dev is setup differently (at least for bugs in Standard).

Quoting Martin van Dijken <sunspam@windgazer.nl>:

> Kris,
> 
> > It looks like most of the speedup of the new version can be 
> > gained by just making the old version take the input length 
> > into account (based on my own limited benchmarking, YMMV):
> 
> Absolutely true, that is the greatest enhancement of the code.
> 
> > public static String escapeXml(String input) {
> >   String output = input;
> >   if (input != null) {
> >     int length = input.length();
> >     StringBuffer sb = new StringBuffer(length);
> >     for (int i = 0; i < length; i++) {
> >       char c = input.charAt(i);
> >       if (c == '&') sb.append("&amp;");
> >       else if (c == '<') sb.append("&lt;");
> >       else if (c == '>') sb.append("&gt;");
> >       else if (c == '"') sb.append("&#034;");
> >       else if (c == '\'') sb.append("&#039;");
> >       else sb.append(c);
> >     }
> >     output = sb.toString();
> >   }
> >   return output;
> > }
> > 
> > I suppose you could also add a test for input.length() == 0. 
> > If the time savings aren't really that significant, I'd opt 
> > for simpler, easier to read code. BTW, any modification 
> > should account for input == null. It doesn't make sense to 
> > throw an NPE.
> 
> Eeeww, I never was much of a tester but to leave that NPE in there:( 
> 
> The time savings for the extra loop are only in the fact that the String
> is never copied char by char to the StringBuffer and then put back into
> a String. I think it's up to David to provide some test cases in which
> this actually is significantly faster. If there are none, I'd have to
> agree with you in choosing code reading above minor performance gain and
> added code bloat.
> 
> > As a point of comparison, here's an equivalent method from 
> > the Struts class TagUtuls (from CVS):
> > 
> > public String filter(String value) {
> > 
> >   if (value == null) {
> >     return (null);
> >   }
> > 
> >   char content[] = new char[value.length()];
> >   value.getChars(0, value.length(), content, 0);
> >   StringBuffer result = new StringBuffer(content.length + 50);
> > 
> >   for (int i = 0; i < content.length; i++) {
> >     switch (content[i]) {
> >       case '<':
> >         result.append("&lt;");
> >         break;
> >       case '>':
> >         result.append("&gt;");
> >         break;
> >       case '&':
> >         result.append("&amp;");
> >         break;
> >       case '"':
> >         result.append("&quot;");
> >         break;
> >       case '\'':
> >         result.append("&#39;");
> >         break;
> >       default:
> >         result.append(content[i]);
> >     }
> >   }
> > 
> >   return result.toString();
> > }
> 
> Such a waste that all this code is developed in parallel. That's
> something that should have a very high priority if you ask me...
> 
> Anyway, thanks for the comments Kris. Is it useful to put your comment
> in the bug I opened in bugzilla as well?
> http://nagoya.apache.org/bugzilla/show_bug.cgi?id=26826
> 
> Martin

-- 
Kris Schneider <mailto:kris@dotech.com>
D.O.Tech       <http://www.dotech.com/>

---------------------------------------------------------------------
To unsubscribe, e-mail: taglibs-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: taglibs-dev-help@jakarta.apache.org


Mime
View raw message