Re: [PATCH 4/8] bundle-uri: download in creationToken order

> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> +struct sorted_bundle_list {
>> +	struct remote_bundle_info **items;
>> +	size_t alloc;
>> +	size_t nr;
>> +};
>> +
>> +static int insert_bundle(struct remote_bundle_info *bundle, void *data)
>> +{
>> +	struct sorted_bundle_list *list = data;
>> +	list->items[list->nr++] = bundle;
>> +	return 0;
>> +}
> 
> Especially given that the type of the list claims it to be "sorted",
> insert_bundle() is a misleading name for a helper that merely
> appends to it to make the list (tentatively) unsorted.
> 
> I am not opposed to "append all to make an unsorted list, then sort
> the list at the end" strategy.

> If I were doing this patch, I would call the type of the list of
> bundles "struct bundle_list" (without "sorted_" in its name), and
> name the variable of that type used here "sorted".  That would make
> it more clear that this particular bundle list starts its life as
> unsorted (with "append_bundle" function adding more elements) and
> then gets sorted in the end, from the above several lines.

>> +static int compare_creation_token(const void *va, const void *vb)
>> +{
>> +	const struct remote_bundle_info * const *a = va;
>> +	const struct remote_bundle_info * const *b = vb;
>> +
>> +	if ((*a)->creationToken > (*b)->creationToken)
>> +		return -1;
>> +	if ((*a)->creationToken < (*b)->creationToken)
>> +		return 1;
>> +	return 0;
>> +}
> 
> Usually compare(a,b) returns the sign of (a-b), but the returned
> value from the above is the opposite.  This is because we want the
> list sorted from newer to older?  It may help developers to name
> such a (reverse) "compare" function differently.