How do I optimize this method for breaking a string in chunks?

Question

Here's the method. I want to know if I am violating any best practices here or if I am doing something wrong as far as the language is concerned.

private List<String> breakStringInChunks(String text, int chunkSize) {
        List<String> chunks = new ArrayList<String>();
        String temporary = "";
        int numberOfChunks = text.length() / chunkSize;
        int beginIndex = 0;
        int endIndex = 0;

        // Add one iteration if numberOfChunks*chunkSize is less than the length of text.
        if ((numberOfChunks * chunkSize) < text.length()) {
            numberOfChunks++;
        }

        // Cut strings and add in the list.
        for (int i = 0; i < numberOfChunks; i++) {
            endIndex+=chunkSize;
            if ((i + 1) == numberOfChunks) {
                temporary = text.substring(beginIndex);
            }
            else {
                temporary = text.substring(beginIndex, endIndex);
            }
            beginIndex=endIndex;
            chunks.add(temporary);
        }

        return chunks;
    }

Answer 1

It's a bit verbose, and there is no need to declare the temporary string at the start of your method, which could make garbage collection a bit slower. The following would be briefer:

private List<String> breakStringInChunks(String text, int chunkSize) {
    int nChunks = (int)Math.ceil(((double)text.length())/chunkSize));
    List<String> chunks = new ArrayList<String>(nChunks);
    // Cut strings and add in the list.
    for (int i = 0; i < text.length(); i+=chunkSize) {
        int endIndex=i+chunksize;
        if (endIndex >= text.length()) {
            chunks.add(text.substring(i));
        } else {
            chunks.add(text.substring(i, endIndex));
        }
    }
    return chunks;
}

One good thing about your method and the text above is that because you always call substring() on the original String, Java will only reference the original character array, so it will save you some memory allocations.

I think the } else { is a more common coding standard for Java.

Answer 2

Unless I'm misunderstanding your intent, it seems like a massive overkill, and uses string creation quite a lot of times, making the algorithm quite inefficient in java, since strings are immutable.

Try this:

public List<String> breakStringsInChunks(String text,int chunkSize) {
    if (chunkSize<=1) {
        throw new IllegalArgumentException("Chunk size must be positive");
    }
    if (text==null || text.isEmpty()) {
        return Collections.emptyList();
    }

    List<String> chunks= new LinkedList<String>();

    int index=0;
    int len = text.length();

    //guaranteed to succeed at least once since 0 length strings we're taken care of
    do {
        chunks.add(text.substring(index, Math.min(index + chunkSize, len)));
        index+=chunkSize;
    } while (index<len);

    return chunks;
}

Answer 3

Briefer still, and avoids potential resizing of the resulting list.

private static List<String> breakStringInChunks(final String text, final int chunkSize) {
    final int numChunks = 0 == (text.length() % chunkSize) ? text.length() / chunkSize : 1 + (text.length() / chunkSize);
    final List<String> chunks = new ArrayList<String>(numChunks);
    for (int startIndex = 0; startIndex < text.length(); startIndex += chunkSize) {
        final int endIndex = Math.min(text.length(), startIndex + chunkSize);
        chunks.add(text.substring(startIndex, endIndex));
    }
    return chunks;
}

Answer 4

public static final List<String> chunk(final String text, final int chunkSize) {
 // Figure out how many chunks we are going to make.
 final int textLength = text.length();
 final int numberOfChunks =
  textLength % chunkSize == 0
  ? textLength / chunkSize
  : textLength / chunkSize + 1;

 // Create an array list of just the right size.
 final ArrayList<String> chunks = new ArrayList<String>(numberOfChunks);

 // Do all the chunking but the last one - here we know that all chunks
 // are exactly chunkSize long.
 for (int i = 0; i < numberOfChunks - 1; i++) {
  chunks.add(text.substring(i * chunkSize, (i + 1) * chunkSize));
 }

 // Add final chunk, which may be shorter than chunkSize, so we use textLength
 // as the end index.
 chunks.add(text.substring((numberOfChunks - 1) * chunkSize, textLength));

 return chunks;
}

Answer 5

Here is my solution. I tried to implement this to be very efficient:

public static List<String> breakStringInChunks(String text, int chunkSize) {
 if (chunkSize < 2) {
  throw new IllegalArgumentException("Chunk size must be > 1");
 }
 if (null == text || text.isEmpty()) {
  return Collections.emptyList();
 }

 List<String> chunks = new ArrayList<String>(1 + (text.length() / chunkSize));

 int length = text.length() - (text.length() % chunkSize);

 for (int i = 0; i < length;) {
  chunks.add(text.substring(i, i += chunkSize));
 }
 if (length < text.length())
  chunks.add(text.substring(length));

 return chunks;
}

Answer 6

How about something like this?

private List<String> breakStringInChunks(String text, int chunkSize)
{
    List<String> chunks = new ArrayList<String>();
    while (text.length() > 0)
    {
        if (chunkSize > text.length())
        {
            chunkSize = text.length();
        }
        chunks.add(text.substring(0, chunkSize));
        text = text.substring(chunkSize);
    }
    return chunks;
}

Answer 7

Here's mine. Not much different from some of the other answers, but test-driven, fwiw.

public class ChunkTest extends TestCase {
    public void testEmpty() throws Exception {
     assertEquals(0, breakStringInChunks("", 1).size());
    }

    public void testOneChunk() throws Exception {
     String s = "abc";
     List<String> chunks = breakStringInChunks(s, s.length());
     assertEquals(s, chunks.get(0));
     assertEquals(1, chunks.size());
    }

    public void testPartialChunk() throws Exception {
     String s = "abc";
     List<String> chunks = breakStringInChunks(s, s.length() + 1);
     assertEquals(s, chunks.get(0));
     assertEquals(1, chunks.size());
    }

    public void testTwoChunks() throws Exception {
     String s = "abc";
     List<String> chunks = breakStringInChunks(s, 2);
     assertEquals("ab", chunks.get(0));
     assertEquals("c", chunks.get(1));
     assertEquals(2, chunks.size());
    }

    public void testTwoEvenChunks() throws Exception {
     String s = "abcd";
     List<String> chunks = breakStringInChunks(s, 2);
     assertEquals("ab", chunks.get(0));
     assertEquals("cd", chunks.get(1));
    }

    private List<String> breakStringInChunks(String text, int chunkSize) {
     if (text.isEmpty())
      return Collections.emptyList();
     int n = (text.length() + chunkSize - 1) / chunkSize;
     List<String> chunks = new ArrayList<String>(n);
     for (int i = 0; i < n; ++i)
      chunks.add(text.substring(i * chunkSize, Math.min((i + 1) * chunkSize, text.length())));
     return chunks;
    }
}