Skip to content

Commit 0270c76

Browse files
authored
Refactoring (#31)
### Build and Continuous Integration - Replaced the Makefile with a bash `make` script. The previous targets were all PHONY, and the Makefile syntax wasn't ideal. - Added GitHub Actions for continuous integration and automation. ### Go Environment and Tools - Integrated Go ecosystem tools for quality assurance, including `mod`, `fmt`, `vet`, `lint`, `staticcheck`, `vulncheck`, and `gogec`. - Updated `go.mod` to Go version 1.18. - Updated Go dependencies. ### Code Refactoring - Simplified node structures and merged the `zeroChild` field into the `children` field. - Simplified code by eliminating bit operations for certain nodes. - Merged similar functionality between the `ForEach` method and the `Iterator.` - Split `node.go` and `tree.go` into separate files for improved modularity. - Refactored and simplified tree iteration and traversal functionality. - Refactored and simplified the `String` method for the tree. ### Testing and Quality Assurance - Introduced golden files for tests. - Added `t.Parallel` for running tests in parallel. - Refactored some tests for better coverage and readability. ### Code Quality - Fixed numerous Go lint warnings. - Added comments to internal code for clarity. - Renamed variables and method names to improve readability. (After all, cache invalidation and naming things are the two hard things in computer science. :) **This refactoring effort enhances code structure and quality without altering the external interface/public API.** --------- Signed-off-by: Pavel Larkin <laxkin@gmail.com>
1 parent bb789fa commit 0270c76

36 files changed

+4054
-2498
lines changed

.github/workflows/go.yml

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,68 @@
1-
# This workflow will build a golang project
2-
# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-go
3-
41
name: Go
52

63
on:
74
push:
8-
branches: [ "master" ]
5+
branches:
6+
- '*'
97
pull_request:
10-
branches: [ "master" ]
8+
branches:
9+
- '*'
1110

1211
jobs:
13-
1412
build:
15-
runs-on: ubuntu-latest
13+
strategy:
14+
matrix:
15+
go: ['stable', 'oldstable']
16+
os: ['ubuntu-latest']
17+
18+
runs-on: ${{ matrix.os }}
19+
20+
name: Go ${{ matrix.go }} in ${{ matrix.os }}
21+
1622
steps:
1723
- uses: actions/checkout@v4
1824

19-
- name: Set up Go
20-
uses: actions/setup-go@v4
25+
- uses: actions/setup-go@v5
26+
name: Install Go
2127
with:
22-
go-version: '1.20'
28+
go-version: ${{ matrix.go }}
29+
check-latest: true
30+
cache: true
31+
32+
- name: Install dependencies
33+
run: go get .
34+
35+
- name: "Run go mod"
36+
run: ./make qa/mod
37+
38+
- name: "Run go fmt"
39+
run: ./make qa/fmt
40+
41+
- name: "Run go vet"
42+
run: ./make qa/vet
43+
44+
- name: "Run go lint"
45+
run: ./make qa/lint
2346

24-
- name: Build
25-
run: go build -v ./...
47+
- name: "Run go staticcheck"
48+
run: ./make qa/staticcheck
49+
50+
- name: "Run go vulncheck"
51+
run: ./make qa/vulncheck
52+
53+
- name: "Run go gosec"
54+
run: ./make qa/gosec
55+
56+
- name: "Run tests"
57+
run: ./make qa/test/cover
58+
59+
- name: "Rename coverage report to art.coverage.${{ matrix.go }}.${{ matrix.os }}.html..."
60+
run: mv ./art.coverage.html art.coverage.${{ matrix.go }}.${{ matrix.os }}.html
61+
62+
- uses: actions/upload-artifact@v4
63+
with:
64+
name: art.coverage.${{ matrix.go }}.${{ matrix.os }}
65+
path: art.coverage.${{ matrix.go }}.${{ matrix.os }}.html
2666

27-
- name: Test
28-
run: go test -v ./...
67+
- name: "Run go benchmarks"
68+
run: ./make qa/benchmarks

.golangci.yaml

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
linters:
2+
enable-all: true
3+
4+
disable:
5+
- testpackage
6+
- ireturn
7+
- varnamelen
8+
- gofumpt
9+
- exhaustruct
10+
11+
issues:
12+
exclude-files:
13+
- tree_dump*
14+
15+
linters-settings:
16+
depguard:
17+
rules:
18+
main:
19+
files:
20+
- $all
21+
- "!$test"
22+
allow:
23+
- $gostd
24+
- github.com/google # all google packages
25+
test:
26+
files:
27+
- "$test"
28+
allow:
29+
- $gostd
30+
- github.com/stretchr
31+
exhaustruct:
32+
exclude:
33+
- ".*_test.go"
34+
35+
output:
36+
sort-results: true
37+
sort-order:
38+
- file # filepath, line, and column.
39+
- severity
40+
- linter

api.go

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package art
22

33
import "errors"
44

5-
// A constant exposing all node types.
5+
// Node types.
66
const (
77
Leaf Kind = 0
88
Node4 Kind = 1
@@ -25,13 +25,18 @@ const (
2525

2626
// These errors can be returned when iteration over the tree.
2727
var (
28-
ErrConcurrentModification = errors.New("Concurrent modification has been detected")
29-
ErrNoMoreNodes = errors.New("There are no more nodes in the tree")
28+
ErrConcurrentModification = errors.New("concurrent modification has been detected")
29+
ErrNoMoreNodes = errors.New("there are no more nodes in the tree")
3030
)
3131

3232
// Kind is a node type.
3333
type Kind int
3434

35+
// String returns string representation of the Kind value.
36+
func (k Kind) String() string {
37+
return []string{"Leaf", "Node4", "Node16", "Node48", "Node256"}[k]
38+
}
39+
3540
// Key Type.
3641
// Key can be a set of any characters include unicode chars with null bytes.
3742
type Key []byte
@@ -100,19 +105,18 @@ type Tree interface {
100105
// Iterator returns an iterator for preorder traversal over leaf nodes by default.
101106
// Pass TraverseXXX as an options to return an iterator for preorder traversal over all NodeXXX types.
102107
Iterator(options ...int) Iterator
103-
//IteratorPrefix(key Key) Iterator
104108

105109
// Minimum returns the minimum valued leaf, true if leaf is found and nil, false otherwise.
106-
Minimum() (min Value, found bool)
110+
Minimum() (Value, bool)
107111

108112
// Maximum returns the maximum valued leaf, true if leaf is found and nil, false otherwise.
109-
Maximum() (max Value, found bool)
113+
Maximum() (Value, bool)
110114

111115
// Returns size of the tree
112116
Size() int
113117
}
114118

115-
// New creates a new adaptive radix tree
119+
// New creates a new adaptive radix tree.
116120
func New() Tree {
117121
return newTree()
118122
}

consts.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,21 @@
11
package art
22

3-
// node constraints
3+
// node constraints.
44
const (
5-
node4Min = 2
6-
node4Max = 4
5+
node4Min = 2 // minimum number of children for node4.
6+
node4Max = 4 // maximum number of children for node4.
77

8-
node16Min = node4Max + 1
9-
node16Max = 16
8+
node16Min = node4Max + 1 // minimum number of children for node16.
9+
node16Max = 16 // maximum number of children for node16.
1010

11-
node48Min = node16Max + 1
12-
node48Max = 48
11+
node48Min = node16Max + 1 // minimum number of children for node48.
12+
node48Max = 48 // maximum number of children for node48.
1313

14-
node256Min = node48Max + 1
15-
node256Max = 256
14+
node256Min = node48Max + 1 // minimum number of children for node256.
15+
node256Max = 256 // maximum number of children for node256.
1616
)
1717

1818
const (
19-
// MaxPrefixLen is maximum prefix length for internal nodes.
20-
MaxPrefixLen = 10
19+
// maxPrefixLen is maximum prefix length for internal nodes.
20+
maxPrefixLen = 10
2121
)

doc.go

Lines changed: 32 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,39 +5,38 @@
55
//
66
// Usage
77
//
8-
// package main
9-
//
10-
// import (
11-
// "fmt"
12-
// "github.com/plar/go-adaptive-radix-tree"
13-
// )
14-
//
15-
// func main() {
16-
//
17-
// tree := art.New()
18-
//
19-
// tree.Insert(art.Key("Hi, I'm Key"), "Nice to meet you, I'm Value")
20-
// value, found := tree.Search(art.Key("Hi, I'm Key"))
21-
// if found {
22-
// fmt.Printf("Search value=%v\n", value)
23-
// }
24-
//
25-
// tree.ForEach(func(node art.Node) bool {
26-
// fmt.Printf("Callback value=%v\n", node.Value())
27-
// return true
28-
// }
29-
//
30-
// for it := tree.Iterator(); it.HasNext(); {
31-
// value, _ := it.Next()
32-
// fmt.Printf("Iterator value=%v\n", value.Value())
33-
// }
34-
// }
35-
//
36-
// // Output:
37-
// // Search value=Nice to meet you, I'm Value
38-
// // Callback value=Nice to meet you, I'm Value
39-
// // Iterator value=Nice to meet you, I'm Value
40-
//
8+
// package main
9+
//
10+
// import (
11+
// "fmt"
12+
// "github.com/plar/go-adaptive-radix-tree"
13+
// )
14+
//
15+
// func main() {
16+
//
17+
// tree := art.New()
18+
//
19+
// tree.Insert(art.Key("Hi, I'm Key"), "Nice to meet you, I'm Value")
20+
// value, found := tree.Search(art.Key("Hi, I'm Key"))
21+
// if found {
22+
// fmt.Printf("Search value=%v\n", value)
23+
// }
24+
//
25+
// tree.ForEach(func(node art.Node) bool {
26+
// fmt.Printf("Callback value=%v\n", node.Value())
27+
// return true
28+
// }
29+
//
30+
// for it := tree.Iterator(); it.HasNext(); {
31+
// value, _ := it.Next()
32+
// fmt.Printf("Iterator value=%v\n", value.Value())
33+
// }
34+
// }
35+
//
36+
// // Output:
37+
// // Search value=Nice to meet you, I'm Value
38+
// // Callback value=Nice to meet you, I'm Value
39+
// // Iterator value=Nice to meet you, I'm Value
4140
//
4241
// Also the current implementation was inspired by [2] and [3]
4342
//

factory.go

Lines changed: 56 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,51 +4,85 @@ import (
44
"unsafe"
55
)
66

7+
// nodeFactory is an interface for creating various types of ART nodes,
8+
// including nodes with different capacities and leaf nodes.
79
type nodeFactory interface {
8-
newNode4() *artNode
9-
newNode16() *artNode
10-
newNode48() *artNode
11-
newNode256() *artNode
12-
newLeaf(key Key, value interface{}) *artNode
10+
newNode4() *nodeRef
11+
newNode16() *nodeRef
12+
newNode48() *nodeRef
13+
newNode256() *nodeRef
14+
15+
newLeaf(key Key, value interface{}) *nodeRef
1316
}
1417

15-
// make sure that objFactory implements all methods of nodeFactory interface
18+
// make sure that objFactory implements all methods of nodeFactory interface.
1619
var _ nodeFactory = &objFactory{}
1720

18-
var factory = newObjFactory()
21+
//nolint:gochecknoglobals
22+
var (
23+
factory = newObjFactory()
24+
)
1925

26+
// newTree creates a new tree.
2027
func newTree() *tree {
21-
return &tree{}
28+
return &tree{
29+
version: 0,
30+
root: nil,
31+
size: 0,
32+
}
2233
}
2334

35+
// objFactory implements nodeFactory interface.
2436
type objFactory struct{}
2537

38+
// newObjFactory creates a new objFactory.
2639
func newObjFactory() nodeFactory {
2740
return &objFactory{}
2841
}
2942

30-
// Simple obj factory implementation
31-
func (f *objFactory) newNode4() *artNode {
32-
return &artNode{kind: Node4, ref: unsafe.Pointer(new(node4))}
43+
// Simple obj factory implementation.
44+
func (f *objFactory) newNode4() *nodeRef {
45+
return &nodeRef{
46+
kind: Node4,
47+
ref: unsafe.Pointer(new(node4)), //#nosec:G103
48+
}
3349
}
3450

35-
func (f *objFactory) newNode16() *artNode {
36-
return &artNode{kind: Node16, ref: unsafe.Pointer(&node16{})}
51+
// newNode16 creates a new node16 as a nodeRef.
52+
func (f *objFactory) newNode16() *nodeRef {
53+
return &nodeRef{
54+
kind: Node16,
55+
ref: unsafe.Pointer(new(node16)), //#nosec:G103
56+
}
3757
}
3858

39-
func (f *objFactory) newNode48() *artNode {
40-
return &artNode{kind: Node48, ref: unsafe.Pointer(&node48{})}
59+
// newNode48 creates a new node48 as a nodeRef.
60+
func (f *objFactory) newNode48() *nodeRef {
61+
return &nodeRef{
62+
kind: Node48,
63+
ref: unsafe.Pointer(new(node48)), //#nosec:G103
64+
}
4165
}
4266

43-
func (f *objFactory) newNode256() *artNode {
44-
return &artNode{kind: Node256, ref: unsafe.Pointer(&node256{})}
67+
// newNode256 creates a new node256 as a nodeRef.
68+
func (f *objFactory) newNode256() *nodeRef {
69+
return &nodeRef{
70+
kind: Node256,
71+
ref: unsafe.Pointer(new(node256)), //#nosec:G103
72+
}
4573
}
4674

47-
func (f *objFactory) newLeaf(key Key, value interface{}) *artNode {
48-
clonedKey := make(Key, len(key))
49-
copy(clonedKey, key)
50-
return &artNode{
75+
// newLeaf creates a new leaf node as a nodeRef.
76+
// It clones the key to avoid any source key mutation.
77+
func (f *objFactory) newLeaf(key Key, value interface{}) *nodeRef {
78+
keyClone := make(Key, len(key))
79+
copy(keyClone, key)
80+
81+
return &nodeRef{
5182
kind: Leaf,
52-
ref: unsafe.Pointer(&leaf{key: clonedKey, value: value}),
83+
ref: unsafe.Pointer(&leaf{ //#nosec:G103
84+
key: keyClone,
85+
value: value,
86+
}),
5387
}
5488
}

0 commit comments

Comments
 (0)