Naresh Jain » Agile FAQs Blog » Managed Chaos » Code should Express Intent and NOT Rely on Side-effects

Code should Express Intent and NOT Rely on Side-effects

Consider the following code

    public static <T> String join(Iterator<T> itr, String seperator) {
        StringBuilder sb = new StringBuilder();
        while (itr.hasNext()) {
            if (sb.length() != 0) {
                sb.append(separator);
            }
            sb.append(itr.next().toString());
        }
        return sb.toString();
    }

When I read through this code, checking StringBuffer’s length and then appending a separator does not communicate the intent very well.

4
5
6

            if (sb.length() != 0) {
                sb.append(separator);
            }

Here we are relying on the side-effect (StringBuffer’s length) instead of asking the iterator. So the code does not flow smoothly. Also, now if you want to add a prefix to the joined String this logic might break.

IMHO, following code solves this problem.

    public static <T> String join(Iterator<T> itr, String seperator) {
        StringBuilder sb = new StringBuilder();
        while (itr.hasNext()) {
            sb.append(itr.next().toString());
            if(itr.hasNext()) {
                sb.append(separator);
            }
        }
        return sb.toString();
    }

This entry was posted on Friday, July 10th, 2009 at 8:14 pm and is filed under Programming. You can follow any responses to this entry through the RSS 2.0 feed. You can skip to the end and leave a response. Pinging is currently not allowed.

Kalpesh

Sorry Naresh. I assume "," as the separator. You can put separator variable instead of ",".

And, sorry for multiple comments.

Kalpesh

Oops, an extra } might break the compilation.
Here is the corrected one.

public static String join(Iterator itr, String seperator) {
StringBuilder sb = new StringBuilder();
if (itr.hasNext())
sb.append(itr.next().toString());

// prefix comma from 2nd item onwards (hence no need to check in the loop)
while (itr.hasNext()) {
sb.append("," + itr.next().toString());
}
return sb.toString();
}

Kalpesh

public static String join(Iterator itr, String seperator) {
StringBuilder sb = new StringBuilder();
if (itr.hasNext())
sb.append(itr.next().toString());

// prefix comma from 2nd item onwards (hence no need to check in the loop)
while (itr.hasNext()) {
sb.append("," + itr.next().toString());
}
}
return sb.toString();
}
Based on quick change I could think of. Not tested ;)

Agile India.

Code should Express Intent and NOT Rely on Side-effects