Re: [PATCH v2 13/17] mktree: use iterator struct to add tree entries to index

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 26, 2024, 21:10 UTC
Message-ID: <xmqq8qyrxveo.fsf@gitster.g>
In-Reply-To: <56f28efff5404a3fa22bd544d6de8ce2d919b78a.1718834285.git.gitgitgadget@gmail.com>

"Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 22 quoted lines

> @@ -157,14 +186,18 @@ static int add_tree_entry_to_index(struct index_state *istate,
>  
>  static void write_tree(struct tree_entry_array *arr, struct object_id *oid)
>  {
> +	struct tree_entry_iterator iter = { NULL };
>  	struct index_state istate = INDEX_STATE_INIT(the_repository);
>  	istate.sparse_index = 1;
>  
>  	sort_and_dedup_tree_entry_array(arr);
>  
> -	/* Construct an in-memory index from the provided entries */
> -	for (size_t i = 0; i < arr->nr; i++) {
> -		struct tree_entry *ent = arr->entries[i];
> +	tree_entry_iterator_init(&iter, arr);
> +
> +	/* Construct an in-memory index from the provided entries & base tree */
> +	while (iter.current) {
> +		struct tree_entry *ent = iter.current;
> +		tree_entry_iterator_advance(&iter);
>  
>  		if (add_tree_entry_to_index(&istate, ent))
>  			die(_("failed to add tree entry '%s'"), ent->name);

OK, looking good.

If we make _iterator_init() and _iterator_advance to both return the current, then the loop can still be like so:

	for (ent = tree_entry_iterator_init(&iter, arr);
             ent;
	     ent = tree_entry_iterator_advance(&iter)) {
		... use ent ...
	}

and .current does not need to be a non-private member, if we wanted to (I am not convinced it is necessarily a better interface to make .current as private---especially if we end up needing _peek() method to learn its value, i.e. the value the most recent call to _init() or _advance() returned. If we need a write access to .current from outside the interator interface, then what I outlined above would not be a good match).

Thanks.