6
\$\begingroup\$

I am new to Java and I am trying to learn about optimizing code to make it production ready. I have the following code below. I would like to know how I could optimize it. I thought by using a small snippet of could, it would be a good way to learn.

Some points:

  1. The function would be run many times to it needs to be fast.
  2. The input is unconstrained since it might come from a user or from a file. Is there a way to deal with this so not exceptions are thrown? I was thinking of using a regular expression.
  3. Is there anything else I need to do to make it production ready? e.g. unit tests. If so, what would be the best way to do this?
  4. I am assuming that the string to be searched is not very long.
  5. When I say optimization, I mean doing things like replacing the + operator with something faster is it can effect memory and performance, etc.
public String strReplave(String originalStr, String oldStr, String newStr) {
    int start = 0;
    while ((start = originalStr.indexOf(oldStr, start)) > 0) {
        originalStr= originalStr.substring(0,start) + newStr+ originalStr.substring(start + oldStr.length());
    start += newStr.length();
}
return originalStr;
}

Thanks for your help and if you need me to clarify anything please let me know.

\$\endgroup\$
  • 3
    \$\begingroup\$ I feel like I'm missing something? Why don't you use String replace(CharSequence target, CharSequence replacement);? Strings implement CharSequence, so you could use that like String ns = "Hello world".replace("world", "people"); \$\endgroup\$ – Corbin Oct 25 '12 at 21:28
5
\$\begingroup\$

String.replace(..) will work as Corbin said.

Be careful when you mix String objects with the '+' operator in Java, especially within a loop.

String s = a + b; 

will be (generally) interpreted as

String s = new StringBuffer(a).append(b).toString();

While not bad on it's own, when performed in a loop, a new StringBuffer (or the newer StringBuilder) object will be created on every loop iteration, which is usually sub-optimal.

When you want to build a String up from pieces, consider storing the result in a StringBuilder object (created outside of the loop) as you go.

\$\endgroup\$
4
\$\begingroup\$

I'd use StringUtils.replace from Apache Commons Lang instead. It's open source, so you can check its source and its unit tests too. StringUtils.replaceEach also could be useful.

By using a standard library, you take advantage of the knowledge of the experts who wrote it and the experience of those who used it before you.

It's from Effective Java, 2nd edition, Item 47: Know and use the libraries where the author mentions only the JDK's built-in libraries but I think the reasoning could be true for other libraries too.

See also: Not Invented Here

\$\endgroup\$
3
\$\begingroup\$

Like Corbin said you have to use replace(..), or replaceAll(..) [native Java functions],
and test the parameters :

public String strReplave(final String originalStr, final String oldStr, String newStr) {
    // add 'final' if parameters are not changed
    if(null == originalStr || originalStr.isEmpty()) {
          return ""; // verify if it is the project solution prerequity
    }
    if(null == oldStr || oldStr.isEmpty()){
        // Send also a  message to Console or API logger 
        return originalStr ;
    }
    if (null == newStr) {
        newStr = ""; // verify if it is the project solution prerequity
    }
    return originalStr.replaceAll(oldStr, newStr);
}
\$\endgroup\$
  • \$\begingroup\$ It's probably a typo: originalStr(replaceAll(oldStr, newStr)) \$\endgroup\$ – palacsint Oct 26 '12 at 11:00
  • 1
    \$\begingroup\$ @palacsint Exact! Thanks for pointing it out \$\endgroup\$ – cl-r Oct 26 '12 at 11:57
  • \$\begingroup\$ I assume this would not take care of especial characters in the string. That they would still through an exception? \$\endgroup\$ – user1775342 Oct 26 '12 at 18:34
  • \$\begingroup\$ A String is null or not; so inside characters can be evaluate and replaced with regex like : Pattern patt = Pattern.compile(regex, flags); String result = patt.matcher(input).replaceAll(replacement); \$\endgroup\$ – cl-r Oct 29 '12 at 8:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.