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

Add PrefixSet.PrefixSeq and PrefixSet.PrefixCompactSeq for Go 1.23+ #12

Closed
database64128 opened this issue Nov 6, 2024 · 2 comments · Fixed by #13
Closed

Add PrefixSet.PrefixSeq and PrefixSet.PrefixCompactSeq for Go 1.23+ #12

database64128 opened this issue Nov 6, 2024 · 2 comments · Fixed by #13

Comments

@database64128
Copy link
Contributor

Serializing large prefix sets will become much more efficient, if we can avoid allocating the intermediate slice by allowing iterator access over the prefixes. I propose we add:

func (s *PrefixSet) PrefixSeq() iter.Seq[netip.Prefix]
func (s *PrefixSet) PrefixCompactSeq() iter.Seq[netip.Prefix]

Alternatively, give the existing Prefixes and PrefixesCompact names to the new iterator methods, to align with the naming conventions.

I'd be happy to work on this and open a PR.

Related proposal at netipx: go4org/netipx#21

@aromatt
Copy link
Owner

aromatt commented Nov 6, 2024

Hi @database64128, thanks for filing this issue. Iterators are definitely on my roadmap for this library. A PR would be much appreciated, if you're up for it.

I'll get back to you about naming (and I'm open to further discussion about it). Relevant context: I have made an effort to mirror the naming patterns of netipx wherever it makes sense. While not perfect, this is part of an explicit goal of netipds to fit in with netip/netipx.

@database64128
Copy link
Contributor Author

Thanks. I went with PrefixSeq, PrefixCompactSeq and opened #13.

database64128 added a commit to database64128/netipds that referenced this issue Dec 19, 2024
database64128 added a commit to database64128/netipds that referenced this issue Dec 20, 2024
database64128 added a commit to database64128/netipds that referenced this issue Dec 20, 2024
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 a pull request may close this issue.

2 participants