gaissmai / bart

The Balanced Routing Table is an adaptation of D. Knuth's ART algorithm combined with popcount level compression and backtracking. It is somewhat slower than ART, but requires considerably less memory.
MIT License
26 stars 3 forks source link

Suggest signature change to WalkXXX() to be compatible with rangefunc iterators #29

Closed tobez closed 4 months ago

tobez commented 4 months ago

If one changes the WalkXXX() signatures to

func (t *Table[V]) Walk(cb func(pfx netip.Prefix, val V) bool)
func (t *Table[V]) Walk4(cb func(pfx netip.Prefix, val V) bool)
func (t *Table[V]) Walk6(cb func(pfx netip.Prefix, val V) bool)

they will become compatible with rangefunc experiment, and if the experiment is enabled (but likely a default one day), syntactic niceties like this become possible:

for pfx, val := range t.Walk {
    ...
}

What do you think?

gaissmai commented 4 months ago

Yes, it would be good to be able to work with (then) standard iterators, but I still need to analyze what that means in more detail. I'm making myself smart, I've only skimmed through it so far and haven't gone any deeper yet. It would be a good opportunity to try it out for myself using a practical example. I'll keep you up to date.

gaissmai commented 4 months ago

nice summary from Eli: https://eli.thegreenplace.net/2023/preview-ranging-over-functions-in-go

again, the question about naming arises.

func (t *Table[V]) Walk(yield func(pfx netip.Prefix, val V) bool) bool
func (t *Table[V]) Walk4(yield func(pfx netip.Prefix, val V) bool) bool
func (t *Table[V]) Walk6(yield func(pfx netip.Prefix, val V) bool) bool

or

func (t *Table[V]) All(yield func(pfx netip.Prefix, val V) bool) bool
func (t *Table[V]) All4(yield func(pfx netip.Prefix, val V) bool) bool
func (t *Table[V]) All6(yield func(pfx netip.Prefix, val V) bool) bool
gaissmai commented 4 months ago

@tobez the devel branch is updated accordingly, please give it a try with gotip. Comments welcome.

gaissmai commented 4 months ago

ah, Eli had a prior version, the signature of the All() function changed, wait, I'll fix the devel branch.

gaissmai commented 4 months ago

Now I have used the correct signatures and tested with the gotip, there is a commented out TestRange.

I don't want to bump up the requirement higher than 1.21 yet.

By the way, go is the best at refactoring

gaissmai commented 4 months ago

devel merged and closed