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

feat: add SliceToSet #514

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

nicklaus-dev
Copy link

see #505

@@ -479,3 +481,17 @@ func ExampleIsSortedByKey() {

// Output: true
}

func ExampleSliceToSet() {
list := []string{"a", "b", "d"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add more test , in which we have duplicate entries in the slice ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I fixed it! Please review it again.

@@ -378,6 +378,17 @@ func SliceToMap[T any, K comparable, V any](collection []T, transform func(item
return Associate(collection, transform)
}

// SliceToSet returns a map with each unique element of the slice as a key.
func SliceToSet[T comparable, Slice ~[]T](collection Slice) map[T]struct{} {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add this function in the Readme.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I fixed it! Please review it again.

@samber
Copy link
Owner

samber commented Aug 20, 2024

Thanks for your first contribution.

I'm not sure this is the best name for such a helper.

1- it is not exactly a "set" data structure, but a map (see https://github.com/emirpasic/gods)
2- it is not self-explaining until you read the function prototype (eg: SliceToMapKey)

Any takes?

Other ideas:

  • make the value type dynamic: lo.SliceToMapKey[int]([]string{"a", "b"}) -> {"a": 0, "b": 0}
  • add a default value as parameter: lo.SliceToXXXX([]string{"a", "b"}, 42) -> {"a": 42, "b": 42}

Before merging this PR, I would like more feedback from the community 🙏

@nicklaus-dev
Copy link
Author

Hi samber,

Thank you for your advice. I understand the concern regarding the SliceToSet potentially causing confusion. I agree that making the method more flexible is important. In some cases, developers might use map[T]bool for convenience when checking for duplicates, while for performance considerations, developers might prefer map[T]struct{}.

To address this, SliceToMapKey might be a solution. This name better reflects the function's purpose and allows for greater flexibility in specifying the value type. Here is the updated implementation:

package main

import "fmt"

// SliceToMapKey returns a map with each unique element of the slice as a key and a default value.
func SliceToMapKey[T comparable, Slice ~[]T, V any](collection Slice, defaultValue V) map[T]V {
	result := make(map[T]V, len(collection))

	for _, item := range collection {
		result[item] = defaultValue
	}

	return result
}

func main() {
	// Example usage:
	// For checking duplicates with map[T]struct{}
	result1 := SliceToMapKey([]string{"a", "b", "a"}, struct{}{})
	fmt.Printf("%v\n", result1) // map[string]struct{}{"a": {}, "b": {}}

	// For checking duplicates with map[T]bool
	result2 := SliceToMapKey([]string{"a", "b", "a"}, true)
	fmt.Printf("%v\n", result2) // map[string]bool{"a": true, "b": true}
}

cc @samber @shivamrazorpay @ccoVeille

@senago
Copy link
Contributor

senago commented Aug 22, 2024

I find naming Keyify optimal
It's a bit hard to read but the intent is clear

func Keyify[T comparable](collection []T) map[T]struct{}

I'd stay with having struct{} as map value simply cause it's looks natural here


Maybe it's even worth adding

func KeyifyBy[T any, K comparable](collection []T, by func(T)K) map[K]T

I could imagine calling it like KeyifyBy(users, user.User.GetID) // map[user.ID]user.User

@nicklaus-dev
Copy link
Author

nicklaus-dev commented Aug 23, 2024

I find naming Keyify optimal It's a bit hard to read but the intent is clear

func Keyify[T comparable](collection []T) map[T]struct{}

I'd stay with having struct{} as map value simply cause it's looks natural here

Maybe it's even worth adding

func KeyifyBy[T any, K comparable](collection []T, by func(T)K) map[K]T

I could imagine calling it like KeyifyBy(users, user.User.GetID) // map[user.ID]user.User

Keyify is good, but KeyifyBy might be SliceToMap which has been implemented. @senago

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.

4 participants