sdk-csharp icon indicating copy to clipboard operation
sdk-csharp copied to clipboard

CloudEventAttribute should implement equality operators

Open jstafford5380 opened this issue 2 years ago • 7 comments

CloudEvent.GetPopulatedAttributes() returns an IEnumerable<KeyValuePair<CloudEventAttribute, object>> which is actually a bit odd. You'd think it would be usable as a dictionary, but I'm guessing there might be a cause where the same attribute names appear more than once, but in my case, I know they won't, so I tried to create a dictionary from it, but unfortunately, since the Key is CloudEventAttribute, indexing it requires equality operators. Even if I simply search the list of KVPs, it makes it awkward to find an attribute that may or may not exist (SingleOrDefault for example, because default is a KVP with null values), so I think I have to loop on it the old fashioned way.

I find the interface in general to be very awkward, but I think some comparison operators would be helpful, or even a helper method that can grab the attribute and/or value by name. There's already a method to get the attribute, there's just no value in it; it's only the name. Maybe there's some other incantation I'm not seeing; the documentation on extension attributes is lacking in general.

jstafford5380 avatar Aug 08 '22 19:08 jstafford5380

I will look at this on Monday when I'm back from holiday. It's relatively awkward to make CloudeventAttribute plenty equality due to constraints - unless we decided to make that not part of the equality.

jskeet avatar Aug 08 '22 19:08 jskeet

Thanks Jon. I wonder if a custom collection object could help. Just spitballing... maybe something that would be used like this:

AttributeCollection col = ce.GetPopulatedAttributeCollection();
IEnumerable<CloudEventAttribute> myAttrs = col.FindByName("myAttr");

It would probably be expected, of course, that the object implements ICollection at the least.

jstafford5380 avatar Aug 12 '22 19:08 jstafford5380

We could add extension methods for that easily without a new collection type. Anyway, will look more carefully on Monday.

jskeet avatar Aug 12 '22 19:08 jskeet

We could add extension methods for that easily without a new collection type. Anyway, will look more carefully on Monday.

This is where my head first to be honest, so I'm with you. I backpedaled to the custom object because it almost felt like there was potentially room for more function around working with those attributes, but in retrospect, those cases probably aren't related to the collection itself.

jstafford5380 avatar Aug 15 '22 15:08 jstafford5380

Looking at this again, while introducing some equality comparers might still be useful, I think for your current use case you could just use:

var attributes = ce.GetPopulatedAttributes().ToDictionary(pair => pair.Key.Name, pair => pair.Value);

That would then give you a Dictionary<string, object> which I suspect is what you want, without any change to the current code at all :)

Does that work for you?

jskeet avatar Aug 15 '22 15:08 jskeet

For this use case, yep this works just fine and is actually how I moved on; I'm good to go. This issue is just meant as an enhancement request for the API

jstafford5380 avatar Aug 15 '22 16:08 jstafford5380

Okay. I'll keep going with the IEqualityComparer PR then. That would allow something like:

Dictionary<CloudEventAttribute, object> dict = new(ce.GetPopulatedAttributes(), CloudEventAttribute.NameComparer);

jskeet avatar Aug 15 '22 16:08 jskeet