nhibernate / nhibernate-core

NHibernate Object Relational Mapper
https://nhibernate.info
GNU Lesser General Public License v2.1
2.11k stars 921 forks source link

Add ToDictionaryAsync and ToHashSetAsync Linq extension methods #3503

Open dennis-gr opened 3 months ago

dennis-gr commented 3 months ago

Convenience methods to avoid the slightly awkward syntax when doing it manually after calling ToListAsync.

Async-Generator fails to generate tests for ToDictionaryAsync, not sure why it doesn't get recognized.

fredericDelaporte commented 2 months ago

What is awkward? (await query.ToListAsync()).ToHashSet()? Or hashSet.UnionWith(await query.ToListAsync())?

I do not see the need.

dennis-gr commented 2 months ago

What is the awkward? (await query.ToListAsync()).ToHashSet()? Or hashSet.UnionWith(await query.ToListAsync())?

Both: the chained method call with the need to add parantheses around the async call, creating the HashSet yourself also isn't great IMHO.

It's a small usability improvement, I don't see the harm in adding these.

fredericDelaporte commented 2 months ago

I see some harm:

dennis-gr commented 2 months ago

I see some harm:

* It may lure users into thinking the thing is optimized in order to directly load the result into a hashset/dictionary instead of going through an intermediate list.

Optimizing this would be possible as soon as https://github.com/nhibernate/nhibernate-core/pull/2274 is done. I'll add a remark that calling these methods still creates an intermediate list.

* That is still some additional code to maintain, just for sparing a couple of parenthesis to callers.

Like I said, ATM it's a small usability improvement that I think is worth it, with the option to provide a slightly optimized version in the future. EF Core has these methods and it would be nice if NH did too.