3
\$\begingroup\$

Given a (long) string src, generate substrings (up to) len characters long on demand by iterating over src and shifting the reading frame by shift characters every time.

StreamSupport.stream works for me, but is there a better (more idiomatic) way to do this, for example, by using a method like Stream.iterate and providing a Predicate and UnaryOperator?

/**
 * Streams strings of length up to len. Each streamed string is created from the string src by removing
 * first "shift" characters and adding last "shift" characters. The shifting starts from 2nd string onward. It's
 * like traversing the given string by a fixed length frame that shifts forward "shift" characters at a time.
 *
 * @param src   the source string, e.g. "abcde"
 * @param len   max length of the string to be streamed, no string streamed from this method will exceed this length, e.g. 2
 * @param shift integer shift-width to traverse src, e.g. 1
 * @return a Stream of strings, e.g. the above will stream "ab", "bc", "cd", and "de"
 */

private static Stream<String> streamStrings(String src, int len, int shift) {
    Iterator<String> traverser = new Iterator<>() {
        private int i = 0, j = Math.min(len, src.length()); // substring(i, j) is what we need

        @Override
        public boolean hasNext() {
            return i < j && j <= src.length();
        }

        @Override
        public String next() {
            String n = src.substring(i, j);
            i += shift;
            if (j < src.length())
                j = Math.min(i + len, src.length());
            else
                j  = i + len;
            return n;
        }
    };
    return StreamSupport.stream(Spliterators.spliteratorUnknownSize(traverser, Spliterator.SIZED), false);
}

Sample Call and Output

streamStrings("abcdef", 3, 2).forEach(System.out::println);
//abc
//cde
//ef
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Hi and welcome to Code Review. I rolled back the last two edits. For one the last edit you made incorporates feedback from an answer you have received. To avoid "answer invalidation", we do not allow such edits here. For more information check out what you may and may not do after receiving answers. I'm also rolling back the title rollback you performed, but I'm not attached to my formulation... The title should describe what the code does, not what your review request is. \$\endgroup\$
    – Vogel612
    Jun 4 at 13:38
3
\$\begingroup\$

You should generally avoid multiple variable initializations on one line:

private int i = 0;
private int j = Math.min(len, src.length());

is more readable.


EDIT: The following suggestion is left in for reference, but is based on an incorrect assumption.

From your sample output I would assume the following desired behaviour as well:

streamStrings("abcdef", 2, 1).forEach(System.out::println);

// ab
// bc
// cd
// de
// ef
// f

which your implementation does not produce. I don't think you need the condition inside next() at all, the following should suffice instead:

j = Math.min(i + len, src.length());
\$\endgroup\$
4
  • \$\begingroup\$ No, once the last character is taken, it should end. ab, bc, cd, de, ef is the desired output for your call (that my code produces). \$\endgroup\$ Jun 4 at 13:08
  • 1
    \$\begingroup\$ Ah I see, then my second point is not applicable for your use case. I would suggest adding comments to the if-block as the way it‘s written might be confusing to some readers (as it was for me). \$\endgroup\$ Jun 4 at 13:23
  • \$\begingroup\$ Done; maybe you can edit your answer as well. Also, looking for the suggestion about a more idiomatic way of doing it. Is my code idiomatic enough (although I see that StringSupport is documented to be useful for library writers). \$\endgroup\$ Jun 4 at 13:29
  • \$\begingroup\$ Will do, once I‘m back at my PC. Sadly I don‘t have a lot of useful advice regarding idiomatic Streams in Java. I hope you‘ll get some useful answers! \$\endgroup\$ Jun 4 at 13:36
3
\$\begingroup\$
  • Braces should always be used even if the body of the if or else contains only a single statement.
  • Missing input validation: shift=0 causes an infinite loop.
  • One variable per declaration: as @riskypenguin also suggested, avoid declarations in one line like int i = 0, j = Math.min(len, src.length());

Using IntStream could be an alternative:

private static Stream<String> streamStrings(String src, int len, int shift) {
    long n = (long) Math.ceil( (double) (src.length() - len) / shift ) + 1;
    return IntStream.range(0, src.length())
            .filter(i -> i % shift == 0)
            .mapToObj(i -> src.substring(i, Math.min(i + len, src.length())))
            .limit(n);
}

Where:

  • n: is the number of strings
  • IntStream.range(0, src.length()).filter(i -> i % shift == 0): is a cluncky way to iterate from 0 to src.length() incrementing by shift. In Python would be range(0, len(src), shift).
  • limit(n): limits the stream to n substrings.
\$\endgroup\$
2
  • 1
    \$\begingroup\$ An alternative way to iterate would be to use String.iterate(0, i -> i + shift). \$\endgroup\$
    – RoToRa
    Jun 7 at 8:23
  • \$\begingroup\$ @RoToRa I don't follow. Can you explain more? \$\endgroup\$ Aug 13 at 20:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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