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\$
1
  • 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
    CommentedOct 25, 2012 at 21:28

3 Answers 3

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\$
      4
      • \$\begingroup\$It's probably a typo: originalStr(replaceAll(oldStr, newStr))\$\endgroup\$
        – palacsint
        CommentedOct 26, 2012 at 11:00
      • 1
        \$\begingroup\$@palacsint Exact! Thanks for pointing it out\$\endgroup\$
        – cl-r
        CommentedOct 26, 2012 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\$CommentedOct 26, 2012 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
        CommentedOct 29, 2012 at 8:41

      Start asking to get answers

      Find the answer to your question by asking.

      Ask question

      Explore related questions

      See similar questions with these tags.