DO NOT REPLY [Bug 36583] New: - Performance modifications on StringUtils.replace

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=36583>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=36583

           Summary: Performance modifications on StringUtils.replace
           Product: Commons
           Version: unspecified
          Platform: Other
        OS/Version: other
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Lang
        AssignedTo: commons-dev@jakarta.apache.org
        ReportedBy: mchyzer@yahoo.com


I dont know how picky you are about performance, but I think it would be nice 
for methods like this not to create new objects (StringBuffers) and copy over, 
if it doesnt need to (e.g. if what you are searching for is not in the "text" 
String).  Also, if the text is empty, we can return it right away.  And, my 
profiler gets two StringBuffers and an arrayCopy here sometimes since we only 
allocate enough for the original String.  I think overall it is better to 
allocate a little more (+20%) for the StringBuffer to reduce this possibility 
and only have 1 StringBuffer to be garbage collected.  Feel free to pick and 
choose from any of these ideas.  :)

public static String replace(String text, String repl, String with, int max) {


could be written as:

  public static String replace(String text, String repl, String with, int max) 
{
    if (isEmpty(text) || isEmpty(repl) || with == null || max == 0) {
      return text;
    }
    int start = 0, end = text.indexOf(repl, start);
    if (end == -1) {
      return text;
    }
    //add a bit of padding so we reduce the StringBuffer extend capacity
    StringBuffer buf = new StringBuffer(text.length() 
        + (text.length() < 20 ? 4 : text.length()/5));
    while (end != -1) {
      buf.append(text.substring(start, end)).append(with);
      start = end + repl.length();

      if (--max == 0) {
        break;
      }
      end = text.indexOf(repl, start);
    }
    buf.append(text.substring(start));
    return buf.toString();
  }


I tested it on the test cases in the javadoc and it worked fine.

Thanks!
Chris

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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