tuple-access: code review fixes
Two significant changes (both arguable, since this patch is about taste by a large measure): 1) Split tuple_seek() into two: tuple_rewind() and tuple_seek(). tuple_rewind() simply initializes the iterator and sets it to 'before-first' position. tuple_seek() returns a field, and sets the iterator to the position after the returned field. Previosly there was only tuple_seek(), which required tuple_next() to get the field, and it was confusing whether or not tuple_next() after tuple_seek() is required. Indeed, the very code I chagned sometimes would call tuple_next() after tuple_seek() and sometimes would not. 2) Move the return value of the iterator outside of iteration state. The advantage of "binding" the out parameters of the iterator to iterator state is that you bind once, and don't need to supply out parameters into every call to tuple_next() function. It also makes it easy to add extra out parameters for bind in the future (field data type, flags, etc). This makes "binding" technique popular in database driver APIs. Binding makes API more stable, which is also important in drivers. In this case, however, binding adds for extra lines of code and (possibly) few extra instructions. Plus, since this is an internal API which we can change any day, its stability is not as important. So it seems that for now we're better of with an API which is a bit more concise/efficient, and can switch to using binding later, if needed.
Showing
- cmake/compiler.cmake 0 additions, 1 deletioncmake/compiler.cmake
- src/box/bitset_index.cc 5 additions, 6 deletionssrc/box/bitset_index.cc
- src/box/box_lua.cc 29 additions, 26 deletionssrc/box/box_lua.cc
- src/box/space.cc 10 additions, 6 deletionssrc/box/space.cc
- src/box/tuple.cc 52 additions, 87 deletionssrc/box/tuple.cc
- src/box/tuple.h 35 additions, 32 deletionssrc/box/tuple.h
- src/memcached-grammar.cc 262 additions, 253 deletionssrc/memcached-grammar.cc
- src/memcached-grammar.rl 12 additions, 9 deletionssrc/memcached-grammar.rl
- src/memcached.cc 12 additions, 16 deletionssrc/memcached.cc
Loading
Please register or sign in to comment