-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dictionary, List and other Collection should have Empty concept #38340
Comments
Tagging subscribers to this area: @eiriktsarpalis |
This was discussed in #24031 and that using System.Collections.Generic;
using System.Collections.Immutable;
public class C {
public void M() {
Foo(ImmutableDictionary<string, int>.Empty);
Foo(ImmutableList<string>.Empty);
}
private static void Foo(IDictionary<string, int> x) {
}
private static void Foo(IList<string> x) {
}
} |
Duplicate of #24031 |
@juliusfriedman you might consider updating the top post to explicitly list the public API you want (eg including HashSet if you want it) so it can be considered together. |
Oh yes. I guess it is rare a empty concrete Dictionary<K,V> is needed. |
@danmosemsft Will do @jkotas I agree in the aspect of Dictionary as proposed it is a duplicate I need to think this through a bit and will update the proposal in due course, I feel that |
My comment overlapped with closing it - I do not have an opinion on whether it is worth reconsidering it. But perhaps the properties on ImmutableDictionary/ImmutableList are not as discoverable as we would like. |
Since we also have I agree that the discover-ability may be slightly less than desired and hence why I mentioned default interface implementations. An alternate design might be an interface Let me know if you need more from me in any regard. Thank you all for your time! |
If you want to pursue this, you'd want to edit the top-post to be the complete proposal, and reopen. |
@danmosemsft, I think I am about done for now, please let me know if you need anything else. I think this may need to be expanded but not quite sure as currently e.g.
Implying as this works:
I also hate to type it here but I also think that even Perhaps an abstract base class like //Could also be DefaultBase
public abstract class NonNullBase<T> where T : class, new {
public static readonly T Default = new T();
public static readonly T Null = null;
public virtual T ToNull() => Null;
public virtual T FromNull() => Default;
} which would bring parity with never having null (such as with struct) but for reference types as well as determining explicitly how to deal with converting to and from null. I think there was a brief conversation I had with @GrabYourPitchforks in a previous thread where I proposed an operator on objects for |
Trying to unpack the OP a bit, it seems to me that the request is to provide "Empty" implementations for The main question then is where should we be hosting these methods? Here's one approach: namespace System.Collections.Generic
{
+ public static class ReadOnlyList
+ {
+ public static IReadOnlyList<T> Empty { get; }
+ }
} Although I'm not too convinced that the added complexity of a new static class is worth it. Alternatively, we could consider adding it as a static property in the interfaces directly, since apparently this is now legal. However, to my knowledge there is no precedent of us doing this in BCL public APIs. |
The search for how to do this leads to the previously closed issue and then to here. ImmutableDictionary is not (in my opinion) discoverable - and I'm even using ImmutableDictionarys in my project. Further the ImmutableDictionary solution doesn't work in same case without a cast to Dictionary (say where a method is expecting the concrete dictionary type). At that point it is kind of getting pretty ugly. var suiteConfigForTool = Suite.ToolConfigs.GetValueOrDefault(Tool.Name, new Dictionary<string, object>(0)); vs (this form is bad) var suiteConfigForTool = Suite.ToolConfigs.GetValueOrDefault(Tool.Name, ImmutableDictionary<string, object>.Empty.ToDictionary()); vs // proposed
var suiteConfigForTool = Suite.ToolConfigs.GetValueOrDefault(Tool.Name, Dictionary<string, object>.Empty)); |
@atruskie I think adding
The easiest expectation to bypass is #2, by changing the |
Hey there, i am not really sure, if some Here is why: 30mins ago, i knew nothing about all that stuff. But i knew using System.Collections.Generic;
using System.Linq;
#nullable enable
namespace MBODM.WADM.Config
{
public record Config
{
public IEnumerable<Game> Games { get; init; } = Enumerable.Empty<Game>();
public IDictionary<string, string> Settings { get; init; } = new Dictionary<string, string>();
}
} Then i thought: In short: All well and good so far. Around this time i realized Next step: Go to interwebz. I read thread. using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
#nullable enable
namespace MBODM.WADM.Config
{
public record Config
{
public IEnumerable<Game> Games { get; init; } = Enumerable.Empty<Game>();
public IDictionary<string, string> Settings { get; init; } = ImmutableDictionary<string, string>.Empty;
}
} A friend of mine, not that deep into C#, inspected above code (accidentally) and asked me: I explained it to him and he was very confused. So he asked me, why there is no I can understand his confusion. 😄 I told him, that the code even could look like this (as mentioned in the previous, closed thread): using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
#nullable enable
namespace MBODM.WADM.Config
{
public record Config
{
public IEnumerable<Game> Games { get; init; } = Enumerable.Empty<Game>();
public IDictionary<string, string> Settings { get; init; } = ImmutableDictionary.Create<string, string>();
}
} There we even explicitly say Side note: Conclusion: Just my 2cents. -- An additional comment from said friend: Was not that easy to disagree. 🥴 |
Why would you expect it to be mutable? |
But you can't expect everyone to abide by that. |
Not sure if i fully get it. But as another one also said: Dictionary<> is mutable. And thats a fact. Imo noone expects anything else. Also point 1: Yes, should indeed return an empty dict in some form. So i dont really see the problem here. Ppl expect from Dictionary<>.Empty() the same they expect from Enumerable<>.Empty(). What do i missing here or what exactly is the problem of „could not satisfied at the same time“ ? |
This is a reference to conflict between the various points. Since
Because |
Ok, get what you mean. Thx. Maybe is there some solution a In my head on one side a Dictionary.Empty is rather useful in complex Linq statements, where you just wanna have the empty dict, so you have something with 0 elements to iterate over. This could be very helpful with Safe-Accses-Stuff (in some form Null Object Pattern) and extension methods, when you dont wanna do complex and ugly null handling with elvis and null coalescing operator (if you know what i mean). On the other side, since its a mutable dict, someone maybe wanna add stuff and so a singleton not works. Maybe it could be done, by return a class implementing IDictionary that way, that the enumeration only uses a singleton, but once the Maybe something like that ? Ofc Overall: I understand the reasons why. I just look at it, from a „conceptual“ point of view and there its easier to understand for most users, when it all uses the same concept. But ofc, if its not possible, its not possible. |
...
... the whole point of LINQ is to iterate over something, so this statement is confusing. From LINQ's point of view, there's nothing special about empty enumerables, or a cached, immutable one. LINQ never mutates anything, so isn't terribly relevant to most of the issues here.
Until null reference types actually cause compile errors, you can't get away from null checks.
It does, actually - the method series is |
|
What about a static property on |
Yeah, but that doesn't prevent library consumers from passing null, so until the runtime/compiler completely enforces it the checks are still required. |
I mean... are we doing it so that we can pass it to something taking the concrete |
It's also not pay-for-play. Every mutating operation on Dictionary would need to check to see if it was the instance / derived type. We're at the point of counting instructions to optimize Dictionary, with an open PR about removing a single null check per operation. Regressing that in the name of this is unlikely to be worthwhile. |
Understand. Thx. Some comments, just for the sake of completeness: I assume the LINQ iteration part was confusing for you, just cause of my bad english. I have serious problems in such discussions to precisely word my thoughts. Sorry! I just tried to say, that some In example, when using Null Object Pattern, to prevent yourself from complex and very hard-to-read LINQ code, heavily spiked with null coalescing and elvis operator. Things along these lines:
or replacing/supporting above code by using Null Object Pattern. In both cases you use Sorry, typed on an iPhone. Can not better explain it at the moment. But its not that terrible important. Was just a use case, where a All in all i understand what you mean and do not really disagree. |
Yeah, can definetely imagine this myself. I slowly come to the conclusion that its some sort of a „natural“ problem. Ppl superficiantly just watch out for some Its just a totally another thing, than Not sure if some straight „empty-concept“ (as mentioned in OP) could be ever well done in framework. Maybe, in regards to actual NRT stuff and the upcoming future, this is something we consider years later from now as something that the language itself has to deal with. Dont know... Maybe Mads has some opinion here on that topic :) |
If you're going to be passing it to LINQ to read from, you could pass |
yep, this seems to work for most cases. |
Stupid question coming to my mind: Why we even watch out for an Empty concept ? I assume for the sole fact, that we knew we have to use something else than Lets just imagine, compiler and/or framework always, for any type of collection, reuse a singleton or cached thing internally when allocating (doing a new), until we write to the memory (by doing an Add-Access i.e.) and then start real writing and allocating. Just internally. Not in the sense that we get another reference before and after we do some .Add(). Not in the sense of above text, when Stephen stated the „pay for play“ part. More in the sense of „we always can just do new() and compiler will do fine and most efficient“. I just wanna say: Maybe this is not a language or framework concern, but the compilers job internally. And we not even should think about it ever. Dont get me wrong. I have zero clue how this all internally works (in heavy contrast to Stephen Toub 😄). This is not a suggestion in any form. I just asked myself, if a C# programmer even should think about performance/allocation, when working with collections. Maybe, in a perfect world, we should just write new List() or new Dict() everywhere, never thinking about Empty concepts and stuff, cause compiler will do it fine for us. But i assume things simply not work this way, otherwise they implemented that years ago. |
Echoing other commenters in the issue, I don't think exposing |
Because all this does is move the "pay for play" check around a bit, and almost certainly isn't worth the effort on the compiler/runtime side. |
yes, indeed. can definetely imagine this by myself. |
Exposing |
|
Background and Motivation
Like
Array.Empty
one typically wants an empty version of a collection. We already have as @vcsjones pointed out below:ImmutableDictionary<TKey,TValue>.Empty Field
ImmutableHashSet.Empty Field
ImmutableList.Empty Field
These are not easily discover-able from the types which most developers start out using e.g.
List<T>
,Dictionary<TKey,TValue>
,HashSet<T>
.There is also the
ISet<T>
Interface to which one can easily useEnumerable.Empty<T>();
to fulfill.ICollection<T>
implementsSystem.Collections.Generic.IEnumerable<T>
and addsCount
andIsReadOnly
One can easily use
System.Collections.Generic.ICollection<int> test = (System.Collections.Generic.ICollection<int>)Array.Empty<int>();
However one cannot use
System.Collections.Generic.ICollection<int> test = (System.Collections.Generic.ICollection<int>)Enumerable.Empty<int>();
as it gives a:See also: Partition.SpeedOpt.cs
EmptyPartition<T>
See also: #27552
Proposed API
namespace System.Collections.Generic { public interface ICollection<T> : System.Collections.Generic.IEnumerable<T>{ + ICollection<T> GetDefaultEmptyCollection() => EmptyPartition<T>.Empty; }
Hopefully this also allows for it to be derived and overridden if required, not sure with DIM's.
Usage Examples
Especially useful once
nullable enable
is applied, you either have to choose to annotate the types that they may return null or you can use theEmpty
Sentinel.Alternative Designs
Default implementation methods on
IDictionary
,EmptyCollections
class or additions to almost all collections.Make
EmptyParition
public and point to it'sEmpty
member explicitly fromICollection
Risks
Low , Similar to someone hijacking new or Array Constructor or patching the Empty method itself.
Benefits
It provides a more straight forward way to create non null returning API's and you don't have to create the Sentinel yourself. Albeit is trivial to have such a
EmptyCollection
class with these required sentinel values.The text was updated successfully, but these errors were encountered: