0

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.

Improve this question
3
  • 1
    Post it at codereview.stackexchange.com. That is the place for optimizing your code. Commented Oct 25, 2012 at 20:08
  • To make it production ready I would suggest using the built-in function. Commented Oct 25, 2012 at 20:08
  • To make it production ready I would suggest you to look into StringBuilder instead of that code you just posted. Commented Oct 25, 2012 at 20:10

5 Answers 5

Reset to default
1

Nothing will beat the built-in method input.replace(old,new), so there is no reason really to try to reimplement it on your own.

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

If the string is very large, would this still be optimal?
@user1775342 you said the string would be smaller.
@user1775342 look into StringBuffer and StringBuilder
You can easily do a timer test by creating a huge string then running both methods (built-in and your own) in separate loops, compare to see which takes lesser time to do same number of iterations.
Yes, no matter how large the string, using replace will still be the best you can get, or very nearly the best.
0 
public static String replace(String originalStr, String oldStr, String newStr) {
    int p = originalStr.indexOf(oldStr);
    if (p == -1) {
        return originalStr;
    } else {
        StringBuilder result = new StringBuilder();
        result.append(originalStr, 0, p);
        result.append(newStr);
        int q = p + oldStr.length();
        while ((p = originalStr.indexOf(oldStr, q)) != -1) {
            result.append(originalStr, q, p);
            result.append(newStr);
            q = p + oldStr.length();
        }
        result.append(originalStr, q, originalStr.length());
        return result.toString();
    }
}
  • Use StringBuilder to concat Strings.
  • Optimization if search string is not found. You can skip this, if this doesn't happen freuently.
Improve this answer

2 Comments

why not StringBuilder or StringBuffer? or string.replace()?
@nosid: I am new to Java. Would you be able to break down what the code is doing, please?
0

Two points:

  1. There are a replaceAll and a replaceFirst methods in the String class (which works with regex), you probably want to use that.
  2. One thing that is highly inefficient in your code is the fact that you concatenate strings with the '+' operator. You should look into the StringBuilder class.
Improve this answer

Comments

0

This is what I use:

public static String replaceString(String cadena, String word, String newWord) {
    Pattern pat = Pattern.compile(word);   
    Matcher mat = pat.matcher(cadena);  
    return mat.replaceAll(newWord); 
}

You don't really need to know Regex to use it because it will match a literal string, like "a literal string" I don't know if it's what you were asking for but works pretty well for me.

Improve this answer

5 Comments

Would this handle unconstrained inputs? The input could be in any format. Since it could come from the user, I am not sure what would cause the code to break.
Well, if you are not sure if the user is gonna use it well, use the string.replace as someone said.
If you care to look at the implementation of String.replace, it does exactly this, but correctly. The difference is precisely in accepting uncostrained strings.
works well for you? If word is ".*" the matcher would match the whole thing and replace it with newWord, none of cadena would be left...
@jlordo and I use it in the right way, always. So I don't have to worry about weird cases because I'm the only one who uses it in my code.
0

Just my two cents besides using the builtin functions. Make it obvious what the code does. Yet include all the possible conditions where your program might fail.

use try{} catch(){exception handling}

measure your code and where the bottle-necks are before trying to optimize the code. Don't guess.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.