Replace any number of consecutive underscores with a single space

Question

I am coding the following interesting problem :

Remove '_' and print with 1 space from a given string For example, for "w_e_b__services" output should be : "w e b services". Multiple underscore should be substituted with only with one space.

The following is my code, which has some code duplication. I can see there are some ways to get rid of that duplication but that complicates the code in a different way. It would be great to get some code review on this.

public class ReplaceUnderscore {

    private static final char UNDERSCORE = '_';
    private static final char SPACE = ' ';

    public static String replace(String input) {

        if (input.isEmpty()) {
            return input;
        }

        StringBuilder output = new StringBuilder();

        char prevChar = input.charAt(0);
        if (prevChar == UNDERSCORE) {
            output.append(SPACE);
        } else {
            output.append(prevChar);
        }

        for (int i = 1; i < input.length(); i++) {
            char currentChar = input.charAt(i);

            if (currentChar == UNDERSCORE) {
                if (currentChar != prevChar) {
                    output.append(SPACE);
                }
            } else {
                output.append(currentChar);
            }

            prevChar = currentChar;
        }

        return output.toString();
    }
}

And here is the junit test:

public class StripUnderscoreTest {

    @Test
    public void strip() throws Exception {

        assertEquals(" ", ReplaceUnderscore.replace("_"));
        assertEquals("a b c", ReplaceUnderscore.replace("a_b_c"));
        assertEquals("a b c", ReplaceUnderscore.replace("a__b__c"));
        assertEquals("a b c", ReplaceUnderscore.replace("a___b___c"));
    }
}

Answer 1

I think overall it is a nice implementation. It is clear and readeable. The use of constants is nice, makes it easy to refactor the code to make it more generic later on (e.g. replace with given character, not just space). Clear variable names (although I'd try and be consistent, and use current\previous, or cur/prev, most likely the former).

I have two remarks:

• I am not a fan of the new line at the start of a function, it is too much, and makes me lose my focus.
• I'd change the check if (currentChar != prevChar) to if (prevChar != UNDERSCORE). Although we can know from the check above that currentChar is an underscore, this makes it even more clear.

I don't really think you have what I'd see as duplication, but a possible other way to implement it would be to keep track of wether you are currently in a sequence of underscores. Example:

boolean previousWasUnderscore = false;
for (int i = 0; i < input.length(); i++) {
    final char currentChar = input.charAt(i);
    if (currentChar == UNDERSCORE) {
        if (!previousWasUnderscore ) {
            output.append(SPACE);
            previousWasUnderscore = true;
        }
    } else {
        output.append(currentChar);
        previousWasUnderscore = false;
    }
}

Answer 2

For generality, I would define a function that accepts the old and new delimiters as parameters, then overload it to make the underscore and space as defaults.

To avoid the special case for input.isEmpty(), I would define a boolean variable to indicate whether the previously encountered character was an underscore. You would also avoid having to re-inspect the previous character to see whether it was an underscore.

You can edit the string in place (using the same buffer for the input and output). It's slightly less cumbersome than calling the StringBuilder methods, in my opinion.

public class ReplaceUnderscore {

    public static String replace(String input) {
        return replace('_', ' ', input);
    }

    public static String replace(char oldDelim, char newDelim, String input) {
        boolean wasOldDelim = false;
        int o = 0;
        char[] buf = input.toCharArray();
        for (int i = 0; i < buf.length; i++) {
           assert(o <= i);
           if (buf[i] == oldDelim) {
               if (wasOldDelim) { continue; }
               wasOldDelim = true;
               buf[o++] = newDelim;
           } else {
               wasOldDelim = false;
               buf[o++] = buf[i];
           }
        }
        return new String(buf, 0, o);
    }
}

Answer 3

You may also find this solution iteresting using an inner empty loop to skip the consecutive underscores. This works because in java assignments like (currentChar = input.charAt(++i)) also have a return value (the assigned value). I also use the pre increment ++i to get the next char index.

for (int i = 0; i < input.length(); i++) {
    final char currentChar = input.charAt(i);
    if (currentChar == UNDERSCORE) {
        while(++i < input.length() && (currentChar = input.charAt(i))==UNDERSCORE)
           ; // skip undercores with empty loop
        output.append(SPACE);
    }
    output.append(currentChar);
}

alternatively you can use a foot controlled while:

for (int i = 0; i < input.length(); i++) {
    final char currentChar = input.charAt(i);
    if (currentChar == UNDERSCORE) {
        do { 
           currentChar = input.charAt(Math.min(++i, input.length()-1))
        while(i < input.length() && currentChar == UNDERSCORE); 
        output.append(SPACE);
    }
    output.append(currentChar);
}