views:

227

answers:

7

I have never found a neat(er) way of doing the following.

Say I have a list/array of strings.

abc
def
ghi
jkl

and I want to concatenate them into a single string delimited by a comma as follows:

abc,def,ghi,jkl

In Java, if I write something like this (pardon the syntax)

String[] list = new String[] {"abc","def","ghi","jkl"};
String str = null;
for (String s : list)
{
   str = str + s + "," ;
}

System.out.println(str);

I'll get

abc,def,ghi,jkl,  //notice the comma in the end

So I have to rewrite the above for loop as follows

... 
for (int i = 0; i < list.length; i++)
{
   str = str + list[i];
   if (i != list.length - 1)
   {
     str = str + ",";
   }
}  
...

My question is , can this be done in a more elegant way in java?

EDIT: I would certainly use a StringBuilder/Buffer for efficiency but wanted to illustrate the case in point without being too verbose. By elegant, I mean a solution that avoids the ugly(?) if check inside the loop.

+2  A: 

Look here:

http://snippets.dzone.com/posts/show/91

for a full discussion of this topic.

ammoQ
Thanks for the link. I am looking for a more elegant approach (perhaps one that involves fewer lines of code). Performance, in my case, is not such a major concern.
Rahul
I don not like checks in the look when you can do it outside
St.Shadow
+2  A: 

Here is my version: Java Tricks: Fastest Way to Collecting Objects in a String

StringBuilder buffer = new StringBuilder ();
String delim = "";
for (Object o: list)
{
    buffer.append (delim);
    delim = ", "; // Avoid if(); assignment is very fast!
    buffer.append (o);
}
buffer.toString ();
Aaron Digulla
It is not very pretty - make an initialization of delim every iteration.
St.Shadow
Please provide your solution in case of deletion of your blog post.
furtelwart
How is if() not fast?
Milan Ramaiya
+1 This is what I do also. Do you have performance figures to prove we are right?
KLE
@Gonzo: if() is slower than an assign. The assignment is just a pointer operation and it always happens with the same address, so the CPU will probably use registers and keep everything in the cache. Also, the code is compact, the setup is constant (no need to do part of the work outside of the loop) and the code in the loop is always the same.
Aaron Digulla
Talking about this degree of performance modifications is ridiculous anyway. Isn't it? Well, then show me a usecase where you need to squeeze out this extra nanosecond ...
sfussenegger
I just did a quick performance test. Statistically speaking, there is no difference whatsoever.Really, an if() is slow?
Milan Ramaiya
@Aaron: I'll agree it's more compact. Faster, no. Even if you were writing in direct assembly, that's not going to make a difference. Furthermore, smart compilers will unroll and optimize it anyways.
Milan Ramaiya
... but to speak of clean code: using `builder.setLength(builder.length() - 1);` after the loop is probably the cleanest (and doesn't require a `System.arraycopy(..)` like `deleteCharAt(int)` does)
sfussenegger
@KLE: No but since my loop doesn't contain conditional code, I'm pretty sure.
Aaron Digulla
@sfussenegger: It might cause the buffer to allocate more memory because that single character doesn't fit which means you have an `arraycopy`.
Aaron Digulla
@Gonzo: if() means a branch and that always costs more, even if you don't notice a difference in runtime. At the very best, the CPU branch prediction hardware must run which at least costs power, leading to heat, leading to premature aging of your CPU. ;) Run this code with Java 1.1 and on a Pentium CPU and it still performs. Run it on a Netbook. Also, the code is very clear, readable and obvious and it's very similar in any language.
Aaron Digulla
@Aaron Digulla: I was comparing setLength(builder.length() - 1) to deleteCharAt(builder.length()) which are doing the same but the former with far less overhead. You're right that an extra character might cause an extra arraycopy - but chances are minimal, especially for Strings of a size where speed really matters (size of the underlying buffer always doubles). But still it's ridiculous to talk about performance at this level anyway. I really don't care if my CPU dies 5 minutes earlier as it wasn't able to predict branches right. Damn, if it wasn't able to do so it deserved it anyway! ;)
sfussenegger
A: 

I would use a StringBuffer to implement this feature. String is immutable so everytime you concat two Strings, a new object is created.

More efficient is the use of StringBuffer:

String[] list = new String[] {"abc","def","ghi","jkl"};
StringBuffer str = new StringBuffer();
for (String s : list) {
   str.append(s);
   str.append(",");
}
str.deleteCharAt(str.length());
System.out.println(str); //automatically invokes StringBuffer.toString();
furtelwart
:p You forgot about last comma...
St.Shadow
Damn right... corrected.
furtelwart
You shouldn't use StringBuffer where it's not necessary, it's thread safe and slow, i would use StringBuilder instead. You also have to delete the last comma
codedevour
+1  A: 
StringBuilder builder = new StringBuilder();
for (String st: list) {
    builder.append(st).append(',');
}
builder.deleteCharAt(builder.length());
String result = builder.toString();

Do not use '+' for string contacenations. It`s slow.

St.Shadow
Missing right parathesis on line 5 ;)
furtelwart
Thanks for correction!
St.Shadow
I like this! =)
codedevour
A: 
for (int i = 0; i < list.length-1; i++)
{
str = str + list[i];
str = str + ",";    
}
str = str + list[list.length-1]
Peter
Using string concatination isn't very fast. This code is also hard to read i think
codedevour
i know string concatenation isent fast thats not the point and yes its less readable just pointing out other options to do what the OP asked
Peter
Have you tried the code with an empty list?
Thorbjørn Ravn Andersen
clearly not did that
Peter
+1  A: 

I'd bet that there are several classes named "StringUtil", "StringsUtil", "Strings" or anything along those lines on the classpath of any medium sized Java project. Most likely, any of them will provide a join function. Here are some examples I've found in my project:

org.apache.commons.lang.StringUtils.join(...)
org.apache.wicket.util.string.Wicket.join(...)
org.compass.core.util.StringUtils.arrayToDelimitedString(...)

As you might want to get rid of some external dependencies in the future, you may want to to something like this:

public static final MyStringUtils {
    private MyStringUtils() {}

    public static String join(Object[] list, String delim) {
        return org.apache.commons.lang.StringUtils.join(list, delim);
    }
}

Now that's what I call "elegant" ;)

sfussenegger
+5  A: 

using google-collections joiner class:

Joiner.on(",").join(list)

done.

Andreas Petersson
+1 for google-collections. Yes, yes, I know -- 'an external dependency just for string joining?!?!' -- but it can make your code so much shorter + more expressive that, if you learn the API, it will pay for itself in an hour. :)
Cowan
once you take a look at the api, you will use ist for much more than just string joining.
Andreas Petersson
@Andreas, suggestions for what else it is good for?
Thorbjørn Ravn Andersen
You right - if external api can be used more that one time - it is best solution.
St.Shadow
@Thorbjorn: see http://www.youtube.com/watch?v=ZeO_J2OcHYM and http://www.youtube.com/watch?v=9ni_KEkHfto for an in-depth explanation why you should use G-C. it simplifies a lot of the boilerplate when using almost any complex data structure in java.
Andreas Petersson
why this is cool: now you want to skip over nulls? Just insert '.skipNulls()' between the 'on' and 'join' calls. Or treat nulls like empty strings? Easy, that's '.useForNull("")'. Etc.
Kevin Bourrillion
Cool - didn't know of this before. Time to throw away our own `StringUtils.join()` utility... :)
Jonik