Optimize this ArrayList join method

Question

I have written this code to join ArrayList elements: Can it be optimized more? Or is there a better different way doing this?

public static String join(ArrayList list, char delim) {
        StringBuffer buf = new StringBuffer();
        for (int i = 0; i < list.size(); i++) {
            if (i != 0)
                buf.append(delim);
            buf.append((String) list.get(i));
        }
        return buf.toString();
    }

Answer 1

In addition to the answers you have received, you can pre-initialize the capacity of the StringBuilder or StringBuffer. This reduces the number of times that the internal array must be recopied as it is expanded. Since you do not know the exact capacity needed you can make an estimate of the average size of each element and multiple by the number of elements.

private static final int ESTIMATE_ELEM_SIZE = 8;

public static String join(Collection col, char delim)
{
    int len = col.size();
    if (len == 0)
        return "";
    StringBuilder buf = new StringBuilder(ESTIMATE_ELEM_SIZE * len);
    for (Object elem : col)
    {
        buf.append(elem).append(delim);
    }
    return buf.substring(0, buf.length() - 1);
}

This shows some techniques others have shared: use StringBuilder, do not put if check within loop, and use foreach enhanced loop.

I would also recommend changing the first parameter to the most generic type that makes sense so it is more reusable. In this case, since you really do not need indexes to address each element, this can specify a Collection.

Note there is also no need to cast to String since StringBuilder.append() will convert the element to a String if necessary. This allows you to use this with collections containing other kinds of objects other than strings.

Answer 2

Why do you need a method at all? Why not just use the toString() method for List?

public static String join(List<String> list) 
{ 
    return list.toString(); // comma delimiter with some extra stuff at start and end
} 

You can do better than the method you wrote. Add the List interface and generics; maybe even Collection. It'll be more generic and type safe; no casts needed.

Run this class:

import java.util.Arrays;
import java.util.List;

public class ListToStringDemo
{
    public static void main(String[] args)
    {
        List<String> values = Arrays.asList(args);
        System.out.println(values);
    }
}

with any string arguments on the command line (e.g., "foo bar baz bat") and get this output:

C:\JDKs\jdk1.6.0_13\bin\java  ListToStringDemo foo bar baz bat
[foo, bar, baz, bat]

Process finished with exit code 0

Answer 3

Here is how the famous java.util.Collection team are doing it, so I'd assume this should be pretty good ;)

  421       /* Returns a string representation of this collection.  The string
  422        * representation consists of a list of the collection's elements in the
  423        * order they are returned by its iterator, enclosed in square brackets
  424        * (<tt>"[]"</tt>).  Adjacent elements are separated by the characters
  425        * <tt>", "</tt> (comma and space).  Elements are converted to strings as
  426        * by {@link String#valueOf(Object)}.
  427        *
  428        * @return a string representation of this collection
  429        */
  430       public String toString() {
  431           Iterator<E> i = iterator();
  432           if (! i.hasNext())
  433               return "[]";
  434   
  435           StringBuilder sb = new StringBuilder();
  436           sb.append('[');
  437           for (;;) {
  438               E e = i.next();
  439               sb.append(e == this ? "(this Collection)" : e);
  440               if (! i.hasNext())
  441                   return sb.append(']').toString();
  442               sb.append(", ");
  443           }

Also, this is how you'll get comma delimiters with duffymo's answer ;)

Answer 4

I doubt it could be significantly optimized (meaning getting faster) without getting arcane. However, it can be improved by using generics.

public static String join(List<String> list, char delim) {
   StringBuffer buf = new StringBuffer(512);
   first = true;
   for (String item : list) {
      if (first) {
         first = false;
      } else {
         buf.append(delim);
      }
      buf.append(item);
   }
}

Improvements in this version: you're enforcing that the item contains only strings at the compiler level, you don't have to maintain a loop variable, and you're not requiring an ArrayList, which is an implementation detail your API shouldn't enforce.

Answer 5

#1

You're checking the size of the list each time your loop iterates:

for (int i = 0; i < list.size(); i++) {

Since you're not altering the list, you only need to do this once:

for (int i = 0, j = list.size(); i < j; i++) {

#2

Instead of checking for i != 0 in each iteration, just append a delimiter after each iteration:

    for (int i = 0; i < list.size(); i++) {
        buf.append((String) list.get(i));
        buf.append(delim);
    }

    // Here, convert all but the last character in the buffer to a string.

Answer 6

Better, maybe:

List instead of ArrayList

Program for interfaces not implementations

for (String string : strings) instead of for (int i ... )

foreach loops from 1.5 are actually more efficient according to Josh, and far easier to grok

etc etc as others have noted

Answer 7

My first thought is that Google Collection's Joiner class would be a useful place to start. Specifically, the public final String join(Iterable<?> parts) method.

Answer 8

StringBuffer is synchronized for thread safety, use a StringBuilder instead.

Don't call list.size() each iteration of the loop. Either set it as a variable or use an Iterator.

Also note that there are lot of libraries for doing this, chiefly google collections. Try the following:

public String join(List<?> list, char delimiter) {
    StringBuilder result = new StringBuilder();
    for (Iterator<?> i = list.iterator(); i.hasNext();) {
        result.append(i.next());
        if (i.hasNext()) {
            result.append(delimiter);
        }
    }
    return result.toString();
}

Answer 9

Is this known known for sure to be causing some performance problem or are you doing this as an exercise? If not, I wouldn't bother.

I suppose this could be faster, but I doubt it:

StringBuffer buf = new StringBuffer();
for (int i = 0; i < list.size() - 1; i++) {
    buf.append((String) list.get(i));
    buf.append(delim);
}
buf.append((String) list.get(i));
return buf.toString();