From: Stefan Beller Date: Tue, 25 Nov 2014 01:16:16 GMT Subject: Re: [PATCH 3/3] string_list: Remove string_list_insert_at_index from its API Message-ID: In-Reply-To: On Mon, Nov 24, 2014 at 4:25 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> This function behaves the same as string_list_insert, just the >> starting indexes for searching, where to insert into the list is also >> a parameter. So if you have knowledge on where to search for the string >> to be inserted, you may have a speed up version of string_list_insert. > > The last half-sentence I am having trouble parsing. If you have > that knowledge where the item should go, you can insert it at the > right location faster than calling string_list_insert()? > > That sounds like a useful feature to me. > > But if nobody uses that feature, there is no point keeping it, no > matter how useful it may sound. So,... I am not sure. > > The function would be error prone if used on a string-list that is > accessed via string_list_insert() API, which makes it only useful if > the user of the API is in full control of the sort order, so I do > not mind removing it and won't be saying "that is useful, we should > keep it and use it more." > > But your log message confuses me.... > Well, I'm sorry for the confusion, here is what I was trying to say: We do not remove functionality from the API, but rather remove a a highly optimized version of a function, which we are not using after patch 2 of the series anyway. We should keep it around only iff we strongly believe there will be a use case later again. But for now I do have the opinion, we're better off removing the function as a smaller API / header file serves us better. Specially new comers may appreciate a smaller API but documented there. Maybe we can drop the commit message altogether if my thoughts are still confusing?