Why does CreateAsync return Task<IEnumerable<TItem>>?

[danielmarbach]

https://github.com/IEvangelist/azure-cosmos-dotnet-repository/blob/main/Microsoft.Azure.CosmosRepository/src/IRepository.cs#L78

return Task<IEnumerable<TItem>> while

https://github.com/IEvangelist/azure-cosmos-dotnet-repository/blob/main/Microsoft.Azure.CosmosRepository/src/IRepository.cs#L64

returns ValueTask<IEnumerable<TItem>> any reasons why they are not alike?

Also, notice the xml doc is not up to date. Happy to send a PR if wanted. It seems though changing the interface return type is a breaking change so I wanted to ask first

[IEvangelist]

Hi @danielmarbach - thank you for posting the issue first, and asking the question. This is by design.

When calls are made to create multiple items at the same time, they're pushed into a Task.WhenAll and each ValueTask calls .AsTask as there is not an equivalent function for ValueTask. I decided to just have that API be represented as Task and left the others as the optimal ValueTask.

[danielmarbach]

I understand that reasons why when doing concurrent processing with WhenAll you need to return Task. I was more wondering from the interface perspective. Because aren't you technically now leaking implementation details to the interface definition?

It seems that the return type is basically bound to the implementation that happens to fan out CreateAsync but from an interface definition perspective, you could also argue that an implementor of the interface could implement this interface internally by using a transactional batch and then there would be mostly synchronous calls to the batch and one ExecuteAsync call which then would allow to essentially return ValueTask again and thus benefit from the value task pooling.

I think my feedback is that it is suprising that the concurrency implementation detail suddendly leaks into the interface definition. From the perspective of the implementation you could also return a ValueTask and then still internally do a when all and be compliant with the Value Task returning API like the following

        public async ValueTask<IEnumerable<TItem>> CreateAsync(IEnumerable<TItem> values)
        {
            IEnumerable<Task<TItem>> concurrentCreationTasks = values.Select(v => CreateAsync(v).AsTask()).ToList();
            await Task.WhenAll(concurrentCreationTasks);
            return concurrentCreationTasks.Select(x => x.Result);
        }

then you allow implementors of the interface to decide how the handle the creation of the multiple items and you are no longer "leaking" the implemenation of the DefaultRepository into the interface definition.

[IEvangelist]

Hi @danielmarbach - I like it and agree with you! Thank you for the suggestion. I'll implement this and make it part of 2.0.0.

[IEvangelist]

Fixed by: db0756f

[danielmarbach]

😍🤩Glad my rambling was helpful