From: Junio C Hamano Date: Tue, 31 May 2016 06:10:08 GMT Subject: Re: [PATCH 09/13] refs: introduce an iterator interface Message-ID: In-Reply-To: <89634d216544d1102dafd5d18247bff2581d48a8.1464537050.git.mhagger@alum.mit.edu> Michael Haggerty writes: > This commit introduces a new iteration primitive for references: a > ref_iterator. A ref_iterator is a polymorphic object that a reference > storage backend can be asked to instantiate. There are three functions > that can be applied to a ref_iterator: > > * ref_iterator_advance(): move to the next reference in the iteration > * ref_iterator_abort(): end the iteration before it is exhausted > * ref_iterator_peel(): peel the reference currently being looked at This part looked somewhat strange in that it makes "peel" sound something very special. Even though I understand why, it made me feel uneasy. I do not think of another operation like peel that may want to have such a specialized helper, so I'll let it pass, but the primary uneasiness I felt comes from the fact that "iterator-peel" is not an essential service of the API that needs for correctness, but is a pure optimization (i.e. you could grab a ref from the normal iterator call, and then ask "please peel this ref" to the usual ref API that does not know anything about iteration). > Iterating using a ref_iterator leaves the flow of control in the hands > of the caller, which means that ref_iterators from multiple > sources (e.g., loose and packed refs) can be composed and presented to > the world as a single compound ref_iterator. Yes, this is a very good move. I am happy to see us going in this direction.