8
\$\begingroup\$

Given a string, S, of length N that is indexed from 0 to N−1, I want to print its even-indexed and odd-indexed characters as 2 space-separated strings on a single line:

Hacker -> Hce akr

My code works, but I feel it could have been done more elegantly and maybe in a single stream instead of in two... Any ideas how to improve this?

private static String reindex(String input) {
    StringBuilder result = new StringBuilder();

    IntStream.range(0, input.length())
             .filter(index -> index % 2 == 0)
             .forEach(index -> result.append(input.charAt(index)));

    result.append(" ");

    IntStream.range(0, input.length())
             .filter(index -> index % 2 == 1)
             .forEach(index -> result.append(input.charAt(index)));

    return result.toString();
}
\$\endgroup\$

2 Answers 2

10
\$\begingroup\$

The main issue with your code is that it operates with side-effects by leveraging the forEach operation. If you were to run your code in parallel, it would be broken and you wouldn't have the expected result. In this case, what you want is to use a mutable reduction approach, that is collect elements into a container.

All propose solutions hardcodes the fact that we reindex even and odd indexes. However, it would be possible to make them generic easily.


The first simple approach is to use 2 Stream pipeline (as you have done). The first one creates the first part of the String and the second one creates the second. Instead of using forEach to add elements to a StringBuilder, we collect the elements into a new StringBuilder. As such, each index is mapped to the character at that index and accumulated into a new StringBuilder with appendCodePoint. The 3 arguments to the collect call corresponds to:

  1. the supplier of the mutable container, (StringBuilder::new);
  2. the accumulator of each element into the container (appending the code point of the character);
  3. a combiner combining two containers into one (used in parallel processing).
private static String reindex(String input) {
    StringBuilder result = new StringBuilder(input.length() + 1);
    result.append(filter(input, index -> index % 2 == 0));
    result.append(' ');
    result.append(filter(input, index -> index % 2 == 1));
    return result.toString();
}

private static StringBuilder filter(String input, IntPredicate predicate) {
    return IntStream.range(0, input.length())
             .filter(predicate)
             .map(input::charAt)
             .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append);
}

The duplicate logic has been extracted to a utility method that filter the indexes matching the given predicate and collects the filtered characters into a StringBuilder.


But the issue is that it loops around the String two times when such an operation should be possible using a single traversal. This is a bit more complicated to do and I would first measure the performance of the first solution before doing this one: it is possible that your concerned String do not have a length big enough to warrant such an approach.

A possible solution is to collect the elements inside an array of StringBuilder. The array to append the current character to is determined by the result of i % 2: when i is even, this will be 0 and it will append it to the first array; when i is odd, it will append it to the second array.

private static String reindex(String input) {
    StringBuilder[] result = 
        IntStream.range(0, input.length())
             .collect(
                () -> new StringBuilder[] { new StringBuilder(), new StringBuilder() },
                (b, i) -> b[i % 2].appendCodePoint(input.charAt(i)),
                (b1, b2) -> {
                    b1[0].append(b2[0]);
                    b1[1].append(b2[1]);
                }
             );

    return result[0] + " " + result[1];
}
\$\endgroup\$
1
  • \$\begingroup\$ Okay. I see I really need to read up about the collect method since I'm not really understanding what happens inside there in the second approach. Seems like the most concise solution, though I would probably try to refactor some of the lambda expressions into separate statements. \$\endgroup\$
    – AdHominem
    Commented May 10, 2016 at 10:35
0
\$\begingroup\$

Add some input validation:

  • Can't reindex a string that's null throw an IllegalArgumentException with a suitable message.
  • Can't add a space to the string if its length is already Integer.MAX_VALUE, again throw an exception as above.
  • Decide if returning a single space is the proper way to handle an empty string.

I would suggest not using streams for this task as they're not really well suited.

Using two simple for-loops, one for even numbers and one for odd, and a StringBuilder is very easy to do and understand.

It's also possible to build the result using only a single iteration of the input string.

Here's how I would write it:

public static String reindex(String input) {
    if(input == null) {
        throw  new IllegalArgumentException("input cannot be null");
    }

    int resultLength = input.length() + 1;
    if(resultLength < 0) {
        throw new IllegalArgumentException("input is too long to be reindexed");
    }

    StringBuilder result = new StringBuilder(resultLength);
    //Setting the length to the same as the initial capacity
    //will only update the internal length property and cause
    //no reallocations or updates to other fields.
    //This allows us to use setCharAt in the loop.
    result.setLength(resultLength);

    int separatorIndex = resultLength / 2;
    int startIndexOfOddParts = separatorIndex + 1;
    result.setCharAt(separatorIndex, ' ');

    for (int index = 0; index < input.length(); index++) {
        int isOdd = index % 2;
        int resultIndex = (index / 2) + startIndexOfOddParts * isOdd;
        result.setCharAt(resultIndex, input.charAt(index));
    }

    return result.toString();
}
\$\endgroup\$

Your Answer

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

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