davidfowl / AspNetCoreDiagnosticScenarios

This repository has examples of broken patterns in ASP.NET Core applications
7.65k stars 736 forks source link

Change Task to ValueTask in ConcurrentDictionary.GetOrAdd example #90

Closed dalibormesaric closed 1 year ago

dalibormesaric commented 1 year ago

I copy/pasted the examples and got compiler errors. Then I checked FindAsync signature, and it expects ValueTask instead of Task, so I thought I could update the document,

https://docs.microsoft.com/en-us/dotnet/api/microsoft.entityframeworkcore.dbcontext.findasync?view=efcore-6.0

davidfowl commented 1 year ago

See https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/pull/75#discussion_r702910191

davidfowl commented 1 year ago

How does ValueTask\<T> help here?

dalibormesaric commented 1 year ago

It just makes it so that the code in the example compiles. But I learned that there is also FindAsync().AsTask() so I can modify my PR to that.

davidfowl commented 1 year ago

Why would we change it to a ValueTask?

dalibormesaric commented 1 year ago

You don't have to change it to ValueTask, it will work with FindAsync().AsTask() instead.

Here is what I'm talking about:

c/p example image with ValueTask image with .AsTask() image

davidfowl commented 1 year ago

Got it, you have a method that didn;t return ValueTask, I understand now. Using AsTask makes sense there. I think we can close this.