lo icon indicating copy to clipboard operation
lo copied to clipboard

feat: add SliceToSet

Open nicklaus-dev opened this issue 1 year ago • 4 comments

see #505

nicklaus-dev avatar Aug 14 '24 16:08 nicklaus-dev

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 🙏

samber avatar Aug 20 '24 22:08 samber

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

nicklaus-dev avatar Aug 21 '24 02:08 nicklaus-dev

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

senago avatar Aug 22 '24 05:08 senago

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

nicklaus-dev avatar Aug 23 '24 03:08 nicklaus-dev

Thanks guy for challenging the name of this helper.

Keyify is very good indeed.

I'm going to make the fix and merge 👍

samber avatar Jan 25 '25 22:01 samber

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.12%. Comparing base (bb32fc7) to head (dd2eaa9). Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #514      +/-   ##
==========================================
+ Coverage   94.10%   94.12%   +0.01%     
==========================================
  Files          18       18              
  Lines        3039     3046       +7     
==========================================
+ Hits         2860     2867       +7     
  Misses        168      168              
  Partials       11       11              
Flag Coverage Δ
unittests 94.12% <100.00%> (+0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 25 '25 22:01 codecov[bot]