-
Notifications
You must be signed in to change notification settings - Fork 2
Feature/curry dictionary updates #85
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
Conversation
Calling on the prefix method now and just checking the arity beforehand
| public abstract NonnegativeInteger Arity { get; } | ||
| public abstract IEnumerable<IList<TKey>> Keyset(); | ||
|
|
||
| public IEnumerable<(IList<TKey>, TVal)> KeyValuePairs() => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is worth adding a couple of tests for these new methods (KeyValuePairs and Values). Just to make sure they return the expected keys and do not fail on empty dictionaries.
| TVal this[params TKey[] keyTuple] { get; } | ||
| IEnumerable<IList<TKey>> Keyset(); | ||
| IEnumerable<(IList<TKey>, TVal)> KeyValuePairs(); | ||
| IEnumerable<TVal> Values(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these be methods or properties? The standard C# dictionaries expose Keys and Values as properties. Is there a particular reason you made these methods instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I originally had Keys as a property, but it felt like a lie to the user as the enumeration was walking through each individual item... However, I suppose that's the way for most enumerable properties... I'll change em to properties so
Added tests for empty dictionary case Added tests for checking enumerables after adding known entries
Description
Fixed bug in the
ContainsKeyTuplemethod