Sunday, 21 August 2005

Spot the 'deliberate' mistake...

I was coding in commons lang earlier tonight when I found one of my test cases wasn't passing. It took a few minutes to realise my stupid error. So here's the test! Can you spot it too???

public int readToken(char[] chars, int start) {
  int pos = start;
  while (pos < chars.length) {
    if (chars[pos++] == '"') {
      return pos;
    }
  }
  return start++;
}

(By the way, there are lots of possible errors here like NPE and this is a very simplified version of the actual method, however somewhere here is a particular 'weird' error.












Did you find it?
Its the return statement. You might think on glancing at the code that the method would return the value of start plus one. But it doesn't. It returns the value of start, as the ++ occurs after the return statement. Obvious really - yet in a non-obvious way. The fix of course is to do return ++start; ;-).

(Sorry if you got it straight away, but it amused me for a minute or two...)

4 comments:

  1. The biggest problem I can see is the never ending while loop - you never increment 'pos'.

    ReplyDelete
  2. Stephen Colebourne21 August 2005 at 14:56

    Sorry my bad :-(

    pos corrected now to pos++

    ReplyDelete
  3. Wouldn't "return start+1" be more clear since you dont use the modified value afterwards?

    ReplyDelete
  4. Stephen Colebourne21 August 2005 at 17:45

    Yes, either
    return ++start;
    or
    return start + 1;
    would work

    And yes, the latter probably is clearer.

    ReplyDelete