Skip to content

fix: order of generics #592

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

Closed
wants to merge 1 commit into from
Closed

Conversation

tarasowski
Copy link

Fix type parameter order in SlicesIndex for clarity

Reordered type parameters so that E is defined before S (~[]E), making the dependency clear. The previous order (S before E) was confusing for beginners, as S depends on E being defined. This improves readability and aligns with best practices in Go generics.

Fix type parameter order in SlicesIndex for clarity

Reordered type parameters so that E is defined before S (~[]E), making the dependency clear. The previous order (S before E) was confusing for beginners, as S depends on E being defined. This improves readability and aligns with best practices in Go generics.
@eliben
Copy link
Collaborator

eliben commented May 6, 2025

What makes you say this aligns with best practices? Can you point to specific examples?

Note that the official slices package is written this way also, e.g. see https://pkg.go.dev/slices#Index
Also the official Go blog posts (e.g. https://go.dev/blog/generic-slice-functions)

@tarasowski
Copy link
Author

As I mentioned in my first issue, it can be a bit confusing for beginners. But since this is the idiomatic way in Go, I'm happy to close the issue.
I was just exploring whether both approaches would work—and they do, at least in the examples I tested (e.g., ...).

func Index[E comparable, S ~[]E](s S, v E) int {
    for i := range s {
        if v == s[i] {
            return i
        }
    }
    return -1
}

@tarasowski tarasowski closed this May 7, 2025
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

Successfully merging this pull request may close these issues.

2 participants