Truncate String to target length using only whole words

Question

I've got a bit of code that I'm wanting to tidy up. The aim is to take a String that consists of multiple words connected by a separator (It's produced using Collectors.joining(SEPARATOR) as below, although the stream processing is not quite so trivial), and return a portion from the front of it.

This portion should never contain partial words, but should in most cases be less than the TARGET_LENGTH number of total characters. It is safe to assume that the words themselves do not contain the SEPARATOR character.

String SEPARATOR = "-";
int TARGET_LENGTH = 20;

//Expected output: "this-is-a-sample"
String s = Arrays.asList("this", "is", "a", "sample", "string")
            .stream()
            .collect(Collectors.joining(SEPARATOR));

if (s.length() > TARGET_LENGTH) {
    if (s.contains(SEPARATOR)) {
        if (s.substring(0, TARGET_LENGTH + 1).contains(SEPARATOR)) {
            return s.substring(0, s.substring(0, TARGET_LENGTH + 1).lastIndexOf(SEPARATOR));
        } else {
            return s.substring(0, s.indexOf(SEPARATOR));
        }
    }
}
return s;

Expected output is as follows:

s                                   ||  result
"easy"                              ||  "easy"
"simple-case"                       ||  "simple-case"
"very-long-input-will-be-truncated" ||  "very-long-input-will"
"countercountermeasures"            ||  "countercountermeasures" //Longer than 20 chars
"countercountermeasures-foo-bar-baz"||  "countercountermeasures" //Longer than 20 chars
"a-b-c-d-e-f-g-veryveryverylongword"||  "a-b-c-d-e-f-g"

The provided code works, but isn't very clear and I imagine it will be horrible to maintain if any requirements change in the future. I'm looking for suggestions as to how it could be improved. I've considered adding a filter to the Stream that generates the String, with a counter keeping track of the current output length declared outside of the string-generation, but this feels like an even worse thing to do (At a guess I'd have to do something silly like update the counter from within a call to Stream.map())

Answer 1

Style

if (s.length() > TARGET_LENGTH) {
    if (s.contains(SEPARATOR)) {
        if (s.substring(0, TARGET_LENGTH + 1).contains(SEPARATOR)) {
            return s.substring(0, s.substring(0, TARGET_LENGTH + 1).lastIndexOf(SEPARATOR));
        } else {
            return s.substring(0, s.indexOf(SEPARATOR));
        }
    }
}

The above code is cumbersome. When you have a return statement inside an if-condition, there is no reason to have an 'else' condition.

Additionally, when you are using early-returns anyway, there's no reason to have such nested code blocks....

Alternatives

I can see multiple options to this problem. I would recommend testing them to see which performs better for you. The Regex option may in fact be faster if the pattern is reused often.

On the fly

In your own answer, you suggest on-the-fly building of the string, and ending early. This is a good option, but I would do it differently:

public static String portion(final int length, final String separator, String...words) {
    if (words.length == 0) {
        return "";
    }

    StringBuilder sb = new StringBuilder(length);
    sb.append(words[0]);
    for (int i = 1; i < words.length; i++) {
        if (sb.length() + separator.length() + words[i].length() > length) {
            return sb.toString();
        }
        sb.append(separator).append(words[i]);
    }
    return sb.toString();
}

Regex

Yeah, yeah, the whole "use a regular expression, now you have two problems...." but, this concept is:

1. If the String is < 20 chars, or has a separator within the 20 chars, take the longest string.
2. If the string is >= 20 or there are no separators within 20, then take until the end of string, or the first separator, whichever is shorter.

If you have the separator - and the length 20, then this can be expressed in the regex:

^(.{0,20}(?=$|-)|.*?(?=$|-))

where:

.{0,20}(?=$|-)

is: find up to 20 chars followed either by the end-of-line, or a separator.

and:

.*?(?=$|-)

is: otherwise, if the previous test fails, take until the end-of-line, or the next separator, whichever happens first.

You can build this in to a reusable pattern (and being defensive about separators that may have special regex meaning....):

private static final Pattern buildPattern(int length, String separator) {
    String qpat = Pattern.quote(separator);
    return Pattern.compile(String.format("^(.{0,%d}(?=$|%s)|.*?(?=$|%s))", length, qpat, qpat));
}

Then, you can use this as follows:

public static String portion(Pattern pat, String s) {
    Matcher mat = pat.matcher(s);
    mat.find(); // patterns will always match, will never fail...
    return mat.group();
}

I imagine this can be used, in your code, like:

private static final Pattern TWENTY_DASH = buildPattern(TARGET_LENGTH, SEPARATOR);

and then in your remaining code:

String s = Arrays.asList("this", "is", "a", "sample", "string")
            .stream()
            .collect(Collectors.joining(SEPARATOR));

return portion(TWENTY_DASH, s);

Return-Early

Having suggested the RegEx option, I would also suggest a simplified lastIndexOf/indexOf using early-returns, instead of nested if-blocks:

public static String portion(final int length, final String separator, String text) {

    if (text.length() <= length) {
        return text;
    }

    int pos = text.lastIndexOf(separator, length);
    if (pos >= 0) {
        return text.substring(0, pos);
    }

    pos = text.indexOf(separator, length);
    if (pos >= 0) {
        return text.substring(0, pos);
    }

    return text;

}

Answer 2

One possible approach that I've just considered is to write it in a style more similar to how I would have done in previous versions of Java, but using the new java.util.StringJoiner and a Stream wrapper around a single word to allow the more elegant way of applying several processing steps before adding it to the result (i.e. avoiding variable declarations for intermediate steps whilst applying processing String lowered = word.toLowerCase(); String processed = foo(lowered);):

String SEPARATOR = "-";
int TARGET_LENGTH = 20;

List<String> words = Arrays.asList("this", "is", "a", "sample", "string");

StringJoiner result = new StringJoiner(SEPARATOR);
for (String word : words) {
    if (result.length() > 0 && TARGET_LENGTH - result.length() < word.length() + 1) {
        break;
    }

    Optional<String> processedWord = Stream.of(word)
            .map(w -> w.toLowerCase(Locale.ENGLISH))
            //Additional processing
            .findFirst();

    if (processedWord.isPresent()) {
        result.add(processedWord.get());
    }
}

return result.toString();

Answer 3

Maybe using String.indexOf would make your life easier:

    String result;

    String SEPARATOR = "-";
    int TARGET_LENGTH = 20;

    int indexToCutFrom = s.indexOf(SEPARATOR, TARGET_LENGTH);
    if (indexToCutFrom >= TARGET_LENGTH)
    {
        result = s.substring(0, indexToCutFrom); 
    }
    else
    {
        indexToCutFrom = s.lastIndexOf(SEPARATOR);
        if (indexToCutFrom != -1)
        {
            result = s.substring(0, indexToCutFrom);
        }
        else
        {
            result = s;
        }
    }

    return result;

Answer 4

Have you considered rolling your own Collector? There is likely to be a more concise way compared to my suggestion below, but that's all I got for now by closely following the implementation of Collectors.joining().

public class StringTruncateCollector implements Collector<CharSequence, StringTruncate, String> {

    private Supplier<StringTruncate> supplier;

    public StringTruncateCollector(final CharSequence delimiter, int maximumLength) {
        this.supplier = () -> new StringTruncate(delimiter, maximumLength);
    }

    @Override
    public Supplier<StringTruncate> supplier() {
        return supplier;
    }

    @Override
    public BiConsumer<StringTruncate, CharSequence> accumulator() {
        return StringTruncate::add;
    }

    @Override
    public BinaryOperator<StringTruncate> combiner() {
        return StringTruncate::merge;
    }

    @Override
    public Function<StringTruncate, String> finisher() {
        return StringTruncate::toString;
    }

    @Override
    public Set<java.util.stream.Collector.Characteristics> characteristics() {
        return Collections.emptySet();
    }

    static final class StringTruncate {
        private StringJoiner joiner;
        private int maximumLength;
        private boolean firstAdded = false;

        public StringTruncate(final CharSequence delimiter, int maximumLength) {
            this.joiner = new StringJoiner(delimiter);
            this.maximumLength = maximumLength;
        }

        public StringTruncate add(CharSequence newElement) {
            if (acceptInput(newElement.length())) {
                joiner.add(newElement);
            }
            firstAdded = true;
            return this;
        }

        public StringTruncate merge(final StringTruncate other) {
            if (acceptInput(Objects.requireNonNull(other).length())) {
                joiner.merge(other.joiner);
            }
            firstAdded = true;
            return this;
        }

        private boolean acceptInput(int incomingLength) {
            return length() + incomingLength <= maximumLength || !firstAdded;
        }

        public int length() {
            return joiner.length();
        }

        @Override
        public String toString() {
            return joiner.toString();
        }
    }
}

StringTruncate is just a simple wrapper for a StringJoiner that checks for the incoming CharSequence length first before calling the methods on the StringJoiner instance. Like I said earlier, I'm not sure at the moment whether there is a more concise way of doing this or not. I also had to rely on a firstAdded boolean value to ensure we have at least the first word as the output.

The test for it:

public class StringTruncateCollectorTest {

    static final String DELIMITER = "-";
    static final int MAX_LENGTH = 20;

    static enum TestCase {
        EASY("easy", "easy"),
        SIMPLE_CASE("simple-case", "simple", "case"),
        LONG("very-long-input-will", "very", "long", "input", "will", "be", "truncated"),
        EXACT("countercountermeasures", "countercountermeasures"),
        TOO_LONG("countercountermeasures", "countercountermeasures", "foo", "bar", "baz"),
        EXTREME("a-b-c-d-e-f-g", "a", "b", "c", "d", "e", "f", "g", "veryveryverylongword");

        private String expected;
        private String[] inputs;

        TestCase(final String expected, final String... inputs) {
            this.expected = expected;
            this.inputs = inputs;
        }

        void verify() {
            assertThat(Stream.of(inputs)
                    .collect(new StringTruncateCollector(DELIMITER, MAX_LENGTH)),
                    equalTo(expected));
        }
    }

    @DataProvider(name = "test-cases")
    public Iterator<Object[]> getTestCases() {
        return Stream.of(TestCase.values()).map(current -> new Object[] { current }).iterator();
    }

    @Test(dataProvider = "test-cases")
    public void testCase(final TestCase current) {
        current.verify();
    }
}

The usage is as follows:

Arrays.asList(...).stream().collect(new StringTruncateCollector(DELIMITER, MAX_LENGTH));