Rework the signature of WalkFn to allow setting new values #3
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request reworks the WalkFn signature to enable modifying node values during tree traversal. Previously, WalkFn returned a boolean to indicate whether to stop walking. Now it returns a (WalkFlag, T) tuple where WalkFlag controls walk behavior (continue, stop, or set) and T is the new value to set when the WalkSet flag is used.
Key changes:
- Introduces
WalkFlagas a bitmask type withWalkContinue,WalkStop, andWalkSetconstants - Updates all
WalkFnimplementations to return(WalkFlag, T)instead ofbool - Adds test coverage for the new
WalkSetfunctionality and flag combinations
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| radix.go | Defines WalkFlag type and constants, updates WalkFn signature, and implements value setting logic in WalkPath, recursiveWalk, and ToMap |
| radix_test.go | Updates all test walk functions to use new signature, adds TestWalkSet to verify value modification, adds TestWalkFlag to verify flag behavior, changes any to string for type consistency |
| go.mod | Moves quicktest from indirect to direct dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Before this commit, you could change the nodes during walk, assuming they were pointers.
But it was not possible to replace a node with a new one (unless they were stored as *interface{}, but that doesn's sound practical).
You could do a new `Insert`, but that be data racy if you e.g. partitioned the tree and walked and modified it concurrently.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The construct in #3 was added to allow setting values while walking but also allow other goroutines to walk the tree concurrently. But that premiss was flawed, as any walk would read the leaf values, hence causing a potential data race. This changes makes the Walk take an interface with an option al pre Check method that takes only the key, and a Handle method that takes both key and value. This allows the caller to skip reading the value if not needed.
1 participant
Before this commit, you could change the nodes during walk, assuming they were pointers.
But it was not possible to replace a node with a new one (unless they were stored as *interface{}, but that doesn's sound practical).
You could do a new
Insert, but that be data racy if you e.g. partitioned the tree and walked and modified it concurrently.