Rotating strings in Java

Question

I have this Java class for producing rotations of an input string:

StringRotator.java

package net.coderodde.util;

import java.util.Objects;

/**
 * This class implements a data type for rotating strings.
 * 
 * @author Rodion "rodde" Efremov
 * @version 1.6 (Mar 24, 2016)
 */
public class StringRotator {

    /**
     * The source string. All rotations will be produced from this string.
     */
    private final String string;

    /**
     * Caches the length of the source string in order to by a little of 
     * efficiency.
     */
    private final int length;

    /**
     * Caches the current rotation. This field is set to {@code null} after 
     * every effective rotation. Whenever the user requests the rotated string,
     * if this field is not {@code null}, the cached string is returned. 
     * Otherwise, a rotated string is built and set to this field, and finally
     * returned.
     */
    private String cachedString;

    /**
     * Index of the first logical character component in the source string.
     */
    private int finger;

    public StringRotator(String string) {
        this.string = Objects.requireNonNull(string, 
                                             "The input string is null.");
        this.length = string.length();
    }

    public void rotate(int offset) {
        int oldFinger = finger;
        finger += offset;
        finger %= string.length();

        if (finger != oldFinger) {
            cachedString = null;
        }
    }

    @Override
    public String toString() {
        if (cachedString != null) {
            return cachedString;
        }

        return cachedString = buildString();
    }

    private String buildString() {
        StringBuilder sb = new StringBuilder(length);

        for (int i = 0; i < length; ++i) {
            sb.append(string.charAt((finger + i) % length));
        }

        return sb.toString();
    }

    public static void main(final String... args) {
        StringBuilder sb = new StringBuilder();

        for (char c = 'a'; c <= 'z'; ++c) {
            sb.append(c);
        }

        String input = sb.toString();

        for (int i = 0; i < input.length(); ++i) {
            StringRotator string = new StringRotator(input);
            string.rotate(i);
            System.out.println(string);
        }
    }
}

Please, tell me anything that comes to mind.

Answer 1

Simplicity

To rotate a string, only the string itself and the offset of rotation are needed.

A class is a way to store data in an orderly manner, but for the task of rotating a string, you do not need such storage.

In Java not needing a class is signalled by the use of static methods.

Also, using String.substring you can avoid the manual looping. In short all your program can be reduced to:

public static String rotate(String s, int offset) {
  int i = offset % s.length();
  return s.substring(i) + s.substring(0, i);
}

The example usage is also simplified, as you do not need to build a new class:

public static void main(final String... args) {
  for (int i = 0; i < 6; i++) {
    System.out.println(rotate("abcdefg", i));
  }
}

The drawback of this approach is that it may be slower and/or use more memory, but unless you want to rotate a string tens of millions of times per second it should not matter.

Answer 2

A couple things:

Your toString seemed backwards to me... and the return a = b(); format struck me as a poor choice. I like the following much better:

public String toString() {
    if (cachedString == null) {
        cachedString = buildString();
    }
    return cachedString;
}

Also, your buildString might be slightly more efficient than mine, but I think mine is a lot more elegant and not significantly slower.

private String buildString() {
    return string.substring(finger) + string.substring(0,finger);
}

I did consider getting rid of buildString altogether, putting that logic in the rotate method and then having toString always return the cache. Up to you.

You optimize by storing final int length but honestly, that's unnecessary. It's already final by being a property of the String (which indeed you use in your rotate method instead of the cached length...) and you can expect it to get JIT'd 110% of the time.

One final note is that your test does the exact same test 26 times. As moderator of Software Quality Assurance Stack Exchange, I hereby give your test driver a D+. (It would have been a D- but seeing the rotated alphabet in my command prompt was oddly satisfying... :-)

Try some other funky tests:

string.rotate(-1);
string.rotate(Integer.MAX_VALUE);
string.rotate(0);
string.rotate(input.length);

Answer 3

I personally don't like classes which encapsulate internally only algorithms but not really needed data (the sign of such classes are -or or -er suffixes). It's actually, sign of procedural programming but not OOP.

So,

1. I would create class with name RotableString and implement for it interface java.lang.CharSequence
2. make the class immutable and don't use any internal cached strings. Only link to the source string and offset as the constructor parameters RotableString(CharSequence source, int offset)
3. if you need to do new rotation from some CharSequence implementation (java.lang.String, your RotableString, java.lang.StringBuilder etc.) just create a new RotableString instance as a decorator with appropriate source and new offset.
4. remove static main method from the class body and add some unit tests instead in a separate file.
5. as a bonus have thread-safety implementation as your class is immutable :)

UPDATE I also suggest not to do any premature optimization till it'll be a real problem and you can confirm it with a profile. See When to optimize paragraph.

Answer 4

Rotating a string as an operation involves appending a substring from the begining of the string to a substring from the start of the string.

This can be stated as taking a substring of the string appended to itself, a fact that one would use if you write a function to determine if a string is a rotation of some other string.

So rather than store the input string you want to store the input string appended to itself.

public StringRotator(String string) {
    this.string = Objects.requireNonNull(string, 
                                         "The input string is null.");
    this.string = this.string.append(string);
    this.length = string.length();
}

You can then write your buildString as

private String buildString() 
{
    return string.substring(finger,length);
}