buger/jsonparser

Dead / Faulty code in ArrayEach?

Bios-Marcel opened this issue · 2 comments

Hey,

I've been trying to use ArrayEach to parse an array. The callback you can pass takes an error value as its argument and can't return one, meaning that even if you want to stop iterating, you can't. Additionally, the passed error can't be non-nil.

v, t, o, e := Get(data[offset:])

if e != nil {
	return offset, e
}

if o == 0 {
	break
}

if t != NotExist {
	cb(v, t, offset+o-len(v), e)
}

if e != nil {
	break
}

If the error is not nil, we'll run into return offset, e. Since e isn't being reassigned before or after the call to cb, as go does't allow this with type error (value type), the if e != nil { break } is dead code and err will always be nil inside of the callback.

What were the thoughts behind the API design of this function? If I don't understand it, I think comments would be really helpful, both inline and function docs.

The API is really uncomfortable to write and I also faced the exact same issue.

I opened the #253 for the same issue and also raised the PR #254 to show a proof of concept.

The API proposed is like

	next, err := jsonparser.ArrayIterator(data, "menu", "items")
	if err != nil {
		// handle error
	}
	for v, t, o, e := next(); e == nil; v, t, o, e = next() {
          // do anything or break when required
	}

If that interests you, you can support & suggest your feedback on that also.

I've also encountered this very nearly bug #230 for additional context since it's not quite obvious.