From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 28 Mar 2019 17:47:48 +0300 From: Vladimir Davydov Subject: Re: [tarantool-patches] Re: [PATCH 3/6] vinyl: cache iterator: consolidate curr_stmt updates Message-ID: <20190328144748.svwojwsniq7q66l6@esperanza> References: <20190328142948.GC23295@chai> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190328142948.GC23295@chai> To: Konstantin Osipov Cc: tarantool-patches@freelists.org List-ID: On Thu, Mar 28, 2019 at 05:29:48PM +0300, Konstantin Osipov wrote: > * Vladimir Davydov [19/03/26 18:56]: > > Currently, vy_cache_iterator->curr_stmt is updated by top-level iterator > > functions - next, skip, restore - which results in code duplication and > > spreads core logic among multiple places. To reduce the amount of code > > and make it generally easier to follow, this patch moves the updates to > > low level functions - step, seek. It also makes the seek method return > > the stop flag, which makes it similar to step, thus making the code more > > consistent. > > OK to push. As a nit, I would pass stop as an out parameter, not > in return value. We may want to use the return value to indicate > an error sometime. Hmm, I just wanted to make it look like vy_cache_iterator_step, but I guess you're right - we'd better avoid using retval as stop indicator. I'll either amend this patch or prepare a separate patch to clean up retvals across all iterators.