Simplify finding indices of all instances of certain characters in a string

Question

I have a string util method for finding potential "wrapping points" in a string.

For example, if " " and "-" are considered wrapping chars, then for input string
"Antoni Gil-Bao" the method would return the sorted set [6, 10].

private static final String WRAPPING_CHARS = " -";
private static final CharMatcher WRAP_CHAR_MATCHER = CharMatcher.anyOf(WRAPPING_CHARS);

public static SortedSet<Integer> findWrappingIndices(String s) {
    SortedSet<Integer> indices = Sets.newTreeSet();
    while (WRAP_CHAR_MATCHER.indexIn(s) != -1) {
        int index = WRAP_CHAR_MATCHER.indexIn(s);
        int indexInOriginalString = indices.isEmpty() ? index :  index + indices.last() + 1;
        indices.add(indexInOriginalString);    
        s = s.substring(index + 1);
    }
    return indices;
}

Question is, is there a simpler way to implement this method using Guava? (Besides trivialities like inlining indexInOriginalString.) Based on my unit tests, the above does the job, but it doesn't feel that elegant. Obviously pure Java solutions are welcome too, if they are simpler than my Guava version. In any case, the method should take a String and return SortedSet<Integer>.

(Background: the reason I just want the indices, instead of doing any string wrapping here directly, is that eventually I'll be operating inside onDraw(), on a char array, and I want to measure pieces of text in the actual font before making wrapping decisions.)

Answer 1

I see two inefficiencies in your code:

• You call .indexIn() twice in quick succession. You should have saved the return value from the first call.
• You create a substring with each iteration. Starting with Java 7, that would be a performance problem, since each substring is a copy of part of the original string.

Also, it would be no additional work for you to generalize the method to accept any CharSequence.

public static SortedSet<Integer> findWrappingIndices(CharSequence s) {
    SortedSet<Integer> indices = Sets.newTreeSet();
    for (int i = -1; -1 != (i = WRAP_CHAR_MATCHER.indexIn(s, i + 1)); ) {
        indices.add(i);    
    }
    return indices;
}

Another option, using just the built-in Java library, would be to use a regex, with [- ] as the pattern.

Answer 2

Simplify is a relative term. If you want simple, then I would have:

public static int[] findWrappingIndices(final String s) {
    int pos = 0;
    int[] ret = new int[s.length()];
    for (int i = 0; i < s.length(); i++) {
        if (WRAPPING_CHARS.indexOf(s.charAt(i)) >= 0) {
            ret[pos++] = i;
        }
    }
    return Arrays.copyOf(ret, pos);
}

But, you want to complexify it by using a SortedSet (because int[] is not easy to use?), so you can then convert the int[] to the SortedSet.

I don't see why the SortedSet is a good idea, but, it is easy enough to convert to using it (even though it is bigger and slower than an int[] array).

I don't think you will find a simpler (or faster) alternative.

I would guess that you could quavify it with (note the data is being inserted in sorted order anyway):

public static SortedSet<Integer> findWrappingIndices(final String s) {
    SortedSet<Integer> indices = Sets.newTreeSet();
    for (int i = 0; i < s.length(); i++) {
        if (WRAPPING_CHARS.indexOf(s.charAt(i)) >= 0) {
            indices.add(i);
        }
    }
    return indices;
}