views:

457

answers:

16

I run into this case a lot of times when doing simple text processing and print statements where I am looping over a collection and I want to special case the last element (for example every normal element will be comma separated except for the last case).

Is there some best practice idiom or elegant form that doesn't require duplicating code or shoving in an if, else in the loop.

For example I have a list of strings that I want to print in a comma separated list. (the do while solution already assumes the list has 2 or more elements otherwise it'd be just as bad as the more correct for loop with conditional).

e.g. List = ("dog", "cat", "bat")

I want to print "[dog, cat, bat]"

I present 2 methods the

  1. For loop with conditional

    public static String forLoopConditional(String[] items) {
    
    
    String itemOutput = "[";
    
    
    for (int i = 0; i < items.length; i++) {
        // Check if we're not at the last element
        if (i < (items.length - 1)) {
            itemOutput += items[i] + ", ";
        } else {
            // last element
            itemOutput += items[i];
        }
    }
    itemOutput += "]";
    
    
    return itemOutput;
     }
    
  2. do while loop priming the loop

    public static String doWhileLoopPrime(String[] items) {
    String itemOutput = "[";
    int i = 0;
    
    
    itemOutput += items[i++];
    if (i < (items.length)) {
        do {
            itemOutput += ", " + items[i++];
        } while (i < items.length);
    }
    itemOutput += "]";
    
    
    return itemOutput;
    }
    

    Tester class:

    public static void main(String[] args) {
        String[] items = { "dog", "cat", "bat" };
    
    
    
    System.out.println(forLoopConditional(items));
    System.out.println(doWhileLoopPrime(items));
    
    }

In the Java AbstractCollection class it has the following implementation (a little verbose because it contains all edge case error checking, but not bad).

public String toString() {
    Iterator<E> i = iterator();
if (! i.hasNext())
    return "[]";

StringBuilder sb = new StringBuilder();
sb.append('[');
for (;;) {
    E e = i.next();
    sb.append(e == this ? "(this Collection)" : e);
    if (! i.hasNext())
    return sb.append(']').toString();
    sb.append(", ");
}
}
+2  A: 

...

String[] items = { "dog", "cat", "bat" };
String res = "[";

for (String s : items) {
   res += (res.length == 1 ? "" : ", ") + s;
}
res += "]";

or so is quite readable. You can put the conditional in a separate if clause, of course. What it makes idiomatic (I think so, at least) is that it uses a foreach loop and does not use a complicated loop header.

Also, no logic is duplicated (i.e. there is only one place where an item from items is actually appended to the output string - in a real world application this might be a more complicated and lengthy formatting operation, so I wouldn't want to repeat the code).

Alexander Gessler
+1 I find this elegant in that testing res.length is a better semantic than detecting the first or the ultimate iteration and behaving differently during a specific iteration. The code above would work even if each iteration had a probability of adding no item to the list.
Heath Hunnicutt
-1 for using a string += in a loop. You've just turned an order n operation in to an order n squared operation as you reconstruct the string every time. :-(
glowcoder
Shit, haven't written Java for years. Thanks for pointing out.
Alexander Gessler
+1  A: 

I like to use a flag for the first item.

 ArrayList<String> list = new ArrayList()<String>{{
       add("dog");
       add("cat");
       add("bat");
    }};
    String output = "[";
    boolean first = true;
    for(String word: list){
      if(!first) output += ", ";
      output+= word;
      first = false;
    }
    output += "]";
Adam
A: 

A third alternative is the following

StringBuilder output = new StringBuilder();
for (int i = 0; i < items.length - 1; i++) {
    output.append(items[i]);
    output.append(",");
}
if (items.length > 0) output.append(items[items.length - 1]);

But the best is to use a join()-like method. For Java there's a String.join in third party libraries, that way your code becomes:

StringUtils.join(items,',');

FWIW, the join() method (line 3232 onwards) in Apache Commons does use an if within a loop though:

public static String join(Object[] array, char separator, int startIndex, int endIndex)     {
        if (array == null) {
            return null;
        }
        int bufSize = (endIndex - startIndex);
        if (bufSize <= 0) {
            return EMPTY;
        }

        bufSize *= ((array[startIndex] == null ? 16 : array[startIndex].toString().length()) + 1);
        StringBuilder buf = new StringBuilder(bufSize);

        for (int i = startIndex; i < endIndex; i++) {
            if (i > startIndex) {
                buf.append(separator);
            }
            if (array[i] != null) {
                buf.append(array[i]);
            }
        }
        return buf.toString();
    }
Vinko Vrsalovic
There is a discussion about java join here: http://stackoverflow.com/questions/794248/a-method-to-reverse-effect-of-java-string-split
Paul Rubel
`output.append(items[i]).append(',');` should be faster because it avoids an intermediate String creation.
Stephen C
The first example will not work on an empty array
finnw
+1  A: 

If you are building a string dynamically like that, you shouldn't be using the += operator. The StringBuilder class works much better for repeated dynamic string concatenation.

public String commaSeparate(String[] items, String delim){
    StringBuilder bob = new StringBuilder();
    for(int i=0;i<items.length;i++){
        bob.append(items[i]);
        if(i+1<items.length){
           bob.append(delim);
        }
    }
    return bob.toString();
}

Then call is like this

String[] items = {"one","two","three"};
StringBuilder bob = new StringBuilder();
bob.append("[");
bob.append(commaSeperate(items,","));
bob.append("]");
System.out.print(bob.toString());
Rambo Commando
A: 

I usually write a for loop like this:

public static String forLoopConditional(String[] items) {
    StringBuilder builder = new StringBuilder();         

    builder.append("[");                                 

    for (int i = 0; i < items.length - 1; i++) {         
        builder.append(items[i] + ", ");                 
    }                                                    

    if (items.length > 0) {                              
        builder.append(items[items.length - 1]);         
    }                                                    

    builder.append("]");                                 

    return builder.toString();                           
}       
frm
If you happen not to know the length ahead of time for whatever reason you could always say add a comma before the item unless it's the first item.
Brian T Hannan
In Java you always know the size of an array. In Java 5 you also know the size of a vararg (like "void functionName(String... args)"), because a vararg is an array with a simplified notation. With Lists you can use the methods size() and get() to achieve the same result. The only problem with collections is using the right algorithm for the used implementation. I.e. my algorithm would be more efficient with an ArrayList, but would be inefficient with a LinkedList.
frm
+3  A: 

I think it is easier to think of the first element as the special case because it is much easier to know if an iteration is the first rather than the last. It does not take any complex or expensive logic to know if something is being done for the first time.

public static String prettyPrint(String[] items) {
    String itemOutput = "[";
    boolean first = true;

    for (int i = 0; i < items.length; i++) {
        if (!first) {
            itemOutput += ", ";
        }

        itemOutput += items[i];
        first = false;
    }

    itemOutput += "]";
    return itemOutput;
}
Jacob Tomaw
yup, that's exactly how I'd do it (except that I'd use `for(String item : items)`). Not necessarily elegant but simple and readable
seanizer
+1  A: 

In this case, you are essentially concatenating a list of strings using some separator string. You can maybe write something yourself which does this. Then you will get something like:

String[] items = { "dog", "cat", "bat" };
String result = "[" + joinListOfStrings(items, ", ") + "]"

with

public static String joinListOfStrings(String[] items, String sep) {
    StringBuffer result;
    for (int i=0; i<items.length; i++) {
        result.append(items[i]);
        if (i < items.length-1) buffer.append(sep);
    }
    return result.toString();
}

If you have a Collection instead of a String[] you can also use iterators and the hasNext() method to check if this is the last or not.

muksie
you should use StringBuilder instead of StringBuffer. It's faster because it avoids the overhead of synchronization
seanizer
+2  A: 

Since your case is simply processing text, you don't need the conditional inside the loop. A C example:

char* items[] = {"dog", "cat", "bat"};
char* output[STRING_LENGTH] = {0};
char* pStr = &output[1];
int   i;

output[0] = '[';
for (i=0; i < (sizeof(items) / sizeof(char*)); ++i) {
    sprintf(pStr,"%s,",items[i]);
    pStr = &output[0] + strlen(output);
}
output[strlen(output)-1] = ']';

Instead of adding a conditional to avoid generating the trailing comma, go ahead and generate it (to keep your loop simple and conditional-free) and simply overwrite it at the end. Many times, I find it clearer to generate the special case just like any other loop iteration and then manually replace it at the end (although if the "replace it" code is more than a couple of lines, this method can actually become harder to read).

bta
It's worth noting that if strings in your language are immutable (C#, Java, etc), you have to take a substring from 0 to len-2 instead of replacing the last char. In those cases, it lacks a certain elegance.
Brian
True, the solution does in part depend on the choice of language. Although if you want elegance, go with the Ruby solution: `["dog","cat","bat"].join(',')`.
bta
I'm puzzled why you used `sprintf` rather than `strcat`. Or better still, `strncat`.
Stephen C
@Stephen C- No real reason, just the first solution I thought of. Definitely not the most efficient.
bta
This solution barfs if you have a zero-length list. Watch those corner cases, that's where security exploits come from.
tylerl
@tylerl- Since all of the arrays are statically declared, I figured it would be safe. If you were using any sort of external input then yes, you would want extra error checking and input validation on everything. My example was just for illustration purposes anyway.
bta
+4  A: 
string value = "[" + StringUtils.join( items, ',' ) + "]";
Jerod Houghtelling
+6  A: 

My usual take is to test if the index variable is zero, e.g.:

var result = "[ ";
for (var i = 0; i < list.length; ++i) {
    if (i != 0) result += ", ";
    result += list[i];
}
result += " ]";

But of course, that's only if we talk about languages that don't have some Array.join(", ") method. ;-)

mishoo
this is the best solution, and the one I always use, but I think your implementation contains a bug... don't you want to check if it's NOT zero?
rmeador
Yeah, fixed that. :-)
mishoo
+15  A: 

I usually write it like this:

static String commaSeparated(String[] items) {
    StringBuilder sb = new StringBuilder();
    String sep = "";
    for (String item: items) {
        sb.append(sep);
        sb.append(item);
        sep = ",";
    }
    return sb.toString();
}
finnw
+1 from me for not adding a new check
ablaeul
I think that's the type of elegance I'm looking for, it took me a second to realize the sep was used to change state as it enters the body of the loop.
Dougnukem
This solution I think is the best because it doesn't contain any conditional check inside the loop instead it just has a repetitive assignment sep = ",", I'd imagine that would make the most efficient solution if you were iterating over a large list.
Dougnukem
@Dougnukem: I think almost all the alternatives presented here will perform very similarly, the JIT would take care of any potentially significant difference. You'd have to make a test to affirm that this is actually the most efficient solution. Not that I dislike this solution, on the contrary.
Vinko Vrsalovic
+1  A: 

I'd go with your second example, ie. handle the special case outside of the loop, just write it a bit more straightforward:

String itemOutput = "[";

if (items.length > 0) {
    itemOutput += items[0];

    for (int i = 1; i < items.length; i++) {
        itemOutput += ", " + items[i];
    }
}

itemOutput += "]";
Secure
A: 

If you are just looking for a comma seperated list of like this: "[The, Cat, in, the, Hat]", don't even waste time writing your own method. Just use List.toString:

List<String> strings = Arrays.asList("The", "Cat", "in", "the", "Hat);

System.out.println(strings.toString());

Provided the generic type of the List has a toString with the value you want to display, just call List.toString:

public class Dog {
    private String name;

    public Dog(String name){
         this.name = name;
    }

    public String toString(){
        return name;
    }
}

Then you can do:

List<Dog> dogs = Arrays.asList(new Dog("Frank"), new Dog("Hal"));
System.out.println(dogs);

And you'll get: [Frank, Hal]

tkeE2036
A: 

Generally, my favourite is the multi-level exit. Change

for ( s1; exit-condition; s2 ) {
    doForAll();
    if ( !modified-exit-condition ) 
        doForAllButLast();
}

to

for ( s1;; s2 ) {
    doForAll();
if ( modified-exit-condition ) break;
    doForAllButLast();
}

It eliminates any duplicate code or redundant checks.

Your example:

for (int i = 0;; i++) {
    itemOutput.append(items[i]);
if ( i == items.length - 1) break;
    itemOutput.append(", ");
}

It works for some things better than others. I'm not a huge fan of this for this specific example.

Of course, it gets really tricky for scenarios where the exit condition depends on what happens in doForAll() and not just s2. Using an Iterator is such a case.

Here's a paper from the prof that shamelessly promoted it to his students :-). Read section 5 for exactly what you're talking about.

Mark Peters
+4  A: 

There are a lot of for loops in these answers, but I find that an Iterator and while loop reads much more easily. E.g.:

Iterator<String> itemIterator = Arrays.asList(items).iterator();
if (itemIterator.hasNext()) {
  // special-case first item.  in this case, no comma
  while (itemIterator.hasNext()) {
    // process the rest
  }
}

This is the approach taken by Joiner in Google collections and I find it very readable.

gk5885
@gk5885 I like this approach because the special case is conditionally handled outside the loop meaning the conditional checking has no effect for large datasets and is trivial and necessary for small data sets.
Dougnukem
A: 

I think there are two answers to this question: the best idiom for this problem in any language, and the best idiom for this problem in java. I also think the intent of this problem wasn't the tasks of joining strings together, but the pattern in general, so it doesn't really help to show library functions that can do that.

Firstly though the actions of surrounding a string with [] and creating a string separated by commas are two separate actions, and ideally would be two separate functions.

For any language, I think the combination of recursion and pattern matching works best. For example, in haskell I would do this:

join [] = ""
join [x] = x
join (x:xs) = concat [x, ",", join xs]

surround before after str = concat [before, str, after]

yourFunc = surround "[" "]" . join

-- example usage: yourFunc ["dog", "cat"] will output "[dog,cat]"

The benefit of writing it like this is it clearly enumerates the different situations that the function will face, and how it will handle it.

Another very nice way to do this is with an accumulator type function. Eg:

join [] = ""
join strings = foldr1 (\a b -> concat [a, ",", b]) strings 

This can be done in other languages as well, eg c#:

public static string Join(List<string> strings)
{
    if (!strings.Any()) return string.Empty;
    return strings.Aggregate((acc, val) => acc + "," + val);
}

Not very efficient in this situation, but can be useful in other cases (or efficiency may not matter).

Unfortunately, java can't use either of those methods. So in this case I think the best way is to have checks at the top of the function for the exception cases (0 or 1 elements), and then use a for loop to handle the case with more than 1 element:

public static String join(String[] items) {
    if (items.length == 0) return "";
    if (items.length == 1) return items[0];

    StringBuilder result = new StringBuilder();
    for(int i = 0; i < items.length - 1; i++) {
        result.append(items[i]);
        result.append(",");
    }
    result.append(items[items.length - 1]);
    return result.toString();
}

This function clearly shows what happens in the two edge cases (0 or 1 elements). It then uses a loop for all but the last elements, and finally adds the last element on without a comma. The inverse way of handling the non-comma element at the start is also easy to do.

Note that the if (items.length == 1) return items[0]; line isn't actually necessary, however I think it makes what the function does more easier to determine at a glance.

(Note that if anyone wants more explanation on the haskell/c# functions ask and I'll add it in)

nanothief