Agile FAQs
  About   Slides   Home  

 
Managed Chaos
Naresh Jain's Random Thoughts on Software Development and Adventure Sports
     
`
 
RSS Feed

Recent Thoughts
Tags
Recent Comments

Code should Express Intent and NOT Rely on Side-effects

Consider the following code

1
2
3
4
5
6
7
8
9
10
    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.

1
2
3
4
5
6
7
8
9
10
    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();
    }
  • 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 ;)

  • 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 ;)

  • 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

    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

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

    And, sorry for multiple comments.

  • Kalpesh

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

    And, sorry for multiple comments.


    Licensed under
Creative Commons License