Skip to content
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

Release Funcky update with .NET 9 and create a new Version 3.5.0 #796

Closed
FreeApophis opened this issue Dec 12, 2024 · 11 comments
Closed

Release Funcky update with .NET 9 and create a new Version 3.5.0 #796

FreeApophis opened this issue Dec 12, 2024 · 11 comments

Comments

@FreeApophis
Copy link
Member

No description provided.

@bash
Copy link
Member

bash commented Jan 7, 2025

Interesting things from the API diff:

  • System.Collections.Generic: There's a new alternate lookup API which comes with new TryGet* methods and an IAlternateEqualityComparer for which we probably want an OptionAlternateEqualityComparer
  • System.Linq: There's a brand-new Index extension method that's functionally the same as our WithIndex.
  • System.Reflection.Metadata has some new TryParse* methods.

@FreeApophis
Copy link
Member Author

FreeApophis commented Jan 7, 2025

And I want to give Priority to the IReadOnlyDictionary overload of GetValueOrNone with the new Overload resolution priority feature.
https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-13.0/overload-resolution-priority

@bash bash mentioned this issue Jan 13, 2025
@FreeApophis
Copy link
Member Author

#798 is merged, is there something missing?

@bash
Copy link
Member

bash commented Jan 14, 2025

The only thing left from my list is IAlternateEqualityComparer but that's not thaat important

@FreeApophis
Copy link
Member Author

Not quite sure what you want to implement for IAlternateEqualityComparer:

  1. Do you want to enable lookup with a T when we have a Dictionary<Option<T>, ...>
  2. Do you want to enable lookup with a Option<T >when we have a Dictionary<T, ...>

Scenario 1 already work due to the implicit cast of the Option.
Scenario 2 not sure if that’s something useful in practice, you can always solve that with a Select.

Example: https://giannisakritidis.com/blog/Alternate-Lookup/

@FreeApophis
Copy link
Member Author

Btw the Dictionary<TKey, TValue>.AlternateLookup<TAlternate> has a TryGetValue method which could use a OrNone-overload.

@bash
Copy link
Member

bash commented Jan 14, 2025

I wanted to enable lookup with an Option<TAlternate> when we have a Dictionary<Option<T>, ..>. Now that I think about it, option keys are pretty rare so I'd leave this part open for the future.

@bash
Copy link
Member

bash commented Jan 15, 2025

I don't like this new alternate lookup API :/

  1. translating the TryGetAlternateLookup function to GetAlternateLookupOrNone results in an extension method with three generic parameters. This makes it basically unusable (due do C#'s lack of partial type inference):
    public static Option<Dictionary<TKey, TValue>.AlternateLookup<TAlternateKey>> GetAlternateLookupOrNone<TKey, TValue, TAlternateKey>(
        this Dictionary<TKey, TValue> dictionary)
        where TKey : notnull
        where TAlternateKey : notnull;
  2. One of the AlternateLookup's TryGetValue is overloaded with varying out parameters:
    public readonly struct AlternateLookup<TAlternateKey> {
        // ...
        public bool TryGetValue(TAlternateKey key, out TKey actualKey, out TValue value);
        public bool TryGetValue(TAlternateKey key, out TValue value);
    }
    Maybe we can give the two overloads distinctive names? I don't think we did that before...
    • TryGetValue
    • TryGetValueAndActualKey?

@FreeApophis
Copy link
Member Author

I needed documentation to even get the idea on what it is... I think the extension is not that important and we might make another 3.x release later this year with new ideas. David pitched a few today.

So should I block the 3.5 release for this?

@bash
Copy link
Member

bash commented Jan 15, 2025

So should I block the 3.5 release for this?

No, I think putting this in another 3.x release (if at all) is better.

David pitched a few today.

I'm excited to see those ideas :)

@FreeApophis
Copy link
Member Author

OK, the prepare commit is merged. I'll create a release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants