Re: [PATCH 3/3] string_list: Remove string_list_insert_at_index from its API
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 25, 2014, 00:25 UTC
- Message-ID
- <xmqq7fykunvs.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1416864124-15231-3-git-send-email-sbeller@google.com>
Stefan Beller <sbeller@google.com> 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....
Show 41 quoted lines
>
> As we're not using this function throughout the codebase, get rid of it.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> string-list.c | 8 +-------
> string-list.h | 2 --
> 2 files changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/string-list.c b/string-list.c
> index c5aa076..9584fa6 100644
> --- a/string-list.c
> +++ b/string-list.c
> @@ -59,13 +59,7 @@ static int add_entry(int insert_at, struct string_list *list, const char *string
>
> struct string_list_item *string_list_insert(struct string_list *list, const char *string)
> {
> - return string_list_insert_at_index(list, -1, string);
> -}
> -
> -struct string_list_item *string_list_insert_at_index(struct string_list *list,
> - int insert_at, const char *string)
> -{
> - int index = add_entry(insert_at, list, string);
> + int index = add_entry(-1, list, string);
>
> if (index < 0)
> index = -1 - index;
> diff --git a/string-list.h b/string-list.h
> index 40ffe0c..ee9b100 100644
> --- a/string-list.h
> +++ b/string-list.h
> @@ -61,8 +61,6 @@ int string_list_find_insert_index(const struct string_list *list, const char *st
> * Returns the string_list_item, the string is part of.
> */
> struct string_list_item *string_list_insert(struct string_list *list, const char *string);
> -struct string_list_item *string_list_insert_at_index(struct string_list *list,
> - int insert_at, const char *string);
>
> /*
> * Checks if the given string is part of a sorted list. If it is part of the list,