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

slices: add CloneFunc function #66404

Closed
apocelipes opened this issue Mar 19, 2024 · 2 comments
Closed

slices: add CloneFunc function #66404

apocelipes opened this issue Mar 19, 2024 · 2 comments
Labels

Comments

@apocelipes
Copy link
Contributor

Proposal Details

I found that there is a common pattern when copying data from slices:

var ss []T
for _, v := range s {
	if conditions {
		ss = append(ss, v)
	}
}

I did a simple scan of the "golang/go" code base, and found more than 30 places of this pattern. For examples:

So I propose to add a simple function "CloneFunc". "CloneFunc" may look like this:

// CloneFunc copies any elements from s for which f returns true
func CloneFunc[S ~[]E, E any](s S, f func(E) bool) S {
	ret := make([]E, 0, len(s)) // Maybe it's not worth doing this
	for i := range s {
		if f(s[i]) {
			ret = append(ret, s[i])
		}
	}
	return ret
}

There are similar functions in other languages:

I personally think "CloneFunc" can make the code simpler, taking "net/lookup.go" as an example:

func (r *Resolver) LookupSRV(ctx context.Context, service, proto, name string) (string, []*SRV, error) {
	...
-	filteredAddrs := make([]*SRV, 0, len(addrs))
-	for _, addr := range addrs {
-		if addr == nil {
-			continue
-		}
-		if !isDomainName(addr.Target) {
-			continue
-		}
-		filteredAddrs = append(filteredAddrs, addr)
-	}
+	filteredAddrs := slices.CloneFunc(addrs, func(addr *SRV) { return addr != nil && isDomainName(addr.Target) } )
	if len(addrs) != len(filteredAddrs) {
		return cname, filteredAddrs, &DNSError{Err: errMalformedDNSRecordsDetail, Name: name}
	}
	return cname, filteredAddrs, nil
}

"CloneFunc" may not be a good name, but it conforms to the naming rules of the slices APIs. Maybe calling it "Filter" would make more sense.

Maybe it is simpler to use loops directly in many cases, but I think it is better to add common patterns to the slices standard library, which can also reduce the difficulty of switching from other languages ​​to golang.

I'm happy to contribute a pull request with tests if this is considered a good idea.

@zephyrtronium
Copy link
Contributor

The conventional name for this operation is filter. The name CloneFunc makes me think of map. And FWIW, both filter and map are special cases of foldMap.

@ianlancetaylor
Copy link
Contributor

Thanks. Closing as dup of #61899 combined with #61898.

@ianlancetaylor ianlancetaylor closed this as not planned Won't fix, can't repro, duplicate, stale Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants