Skip to content

Commit dd9dfea

Browse files
committed
Fix bugs
1 parent bcd47b8 commit dd9dfea

File tree

3 files changed

+98
-46
lines changed

3 files changed

+98
-46
lines changed

src/ordered/btree_map.zig

Lines changed: 56 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,28 @@ pub fn BTreeMap(
7777
return null;
7878
}
7979

80-
/// Inserts a key-value pair. If the key exists, the value is updated.
81-
pub fn put(self: *Self, key: K, value: V) !void {
82-
// Check if key exists and just update the value in place
83-
if (self.get(key) != null) {
84-
_ = self.remove(key);
85-
// Don't increment len here, it will be incremented below
80+
/// Retrieves a mutable pointer to the value associated with `key`.
81+
pub fn getPtr(self: *Self, key: K) ?*V {
82+
var current = self.root;
83+
while (current) |node| {
84+
const res = std.sort.binarySearch(K, node.keys[0..node.len], key, compareFn);
85+
if (res) |index| return &node.values[index];
86+
87+
if (node.is_leaf) return null;
88+
89+
const insertion_point = std.sort.lowerBound(K, node.keys[0..node.len], key, compareFn);
90+
current = node.children[insertion_point];
8691
}
92+
return null;
93+
}
94+
95+
/// Returns true if the map contains the given key.
96+
pub fn contains(self: *const Self, key: K) bool {
97+
return self.get(key) != null;
98+
}
8799

100+
/// Inserts a key-value pair. If the key exists, the value is updated.
101+
pub fn put(self: *Self, key: K, value: V) !void {
88102
var root_node = if (self.root) |r| r else {
89103
const new_node = try self.createNode();
90104
new_node.keys[0] = key;
@@ -104,8 +118,10 @@ pub fn BTreeMap(
104118
root_node = new_root;
105119
}
106120

107-
self.insertNonFull(root_node, key, value);
108-
self.len += 1;
121+
const is_new = self.insertNonFull(root_node, key, value);
122+
if (is_new) {
123+
self.len += 1;
124+
}
109125
}
110126

111127
fn splitChild(self: *Self, parent: *Node, index: u16) void {
@@ -153,25 +169,47 @@ pub fn BTreeMap(
153169
parent.len += 1;
154170
}
155171

156-
fn insertNonFull(self: *Self, node: *Node, key: K, value: V) void {
172+
fn insertNonFull(self: *Self, node: *Node, key: K, value: V) bool {
157173
var i = node.len;
158174
if (node.is_leaf) {
175+
// Check if key already exists
176+
var j: u16 = 0;
177+
while (j < node.len) : (j += 1) {
178+
if (compare(key, node.keys[j]) == .eq) {
179+
// Update existing value
180+
node.values[j] = value;
181+
return false; // Not a new insertion
182+
}
183+
}
184+
185+
// Insert new key
159186
while (i > 0 and compare(key, node.keys[i - 1]) == .lt) : (i -= 1) {
160187
node.keys[i] = node.keys[i - 1];
161188
node.values[i] = node.values[i - 1];
162189
}
163190
node.keys[i] = key;
164191
node.values[i] = value;
165192
node.len += 1;
193+
return true; // New insertion
166194
} else {
195+
// Check if key exists in current node
196+
var j: u16 = 0;
197+
while (j < node.len) : (j += 1) {
198+
if (compare(key, node.keys[j]) == .eq) {
199+
// Update existing value
200+
node.values[j] = value;
201+
return false; // Not a new insertion
202+
}
203+
}
204+
167205
while (i > 0 and compare(key, node.keys[i - 1]) == .lt) : (i -= 1) {}
168206
if (node.children[i].?.len == BRANCHING_FACTOR - 1) {
169207
self.splitChild(node, i);
170208
if (compare(node.keys[i], key) == .lt) {
171209
i += 1;
172210
}
173211
}
174-
self.insertNonFull(node.children[i].?, key, value);
212+
return self.insertNonFull(node.children[i].?, key, value);
175213
}
176214
}
177215

@@ -227,18 +265,23 @@ pub fn BTreeMap(
227265
const pred = self.getPredecessor(node, index);
228266
node.keys[index] = pred.key;
229267
node.values[index] = pred.value;
268+
// deleteFromNode already decremented self.len, so increment it back
269+
// because we're replacing, not actually removing
270+
const old_len = self.len;
230271
_ = self.deleteFromNode(node.children[index].?, pred.key);
231-
self.len += 1;
272+
self.len = old_len;
232273
} else if (node.children[index + 1].?.len > MIN_KEYS) {
233274
const succ = self.getSuccessor(node, index);
234275
node.keys[index] = succ.key;
235276
node.values[index] = succ.value;
277+
const old_len = self.len;
236278
_ = self.deleteFromNode(node.children[index + 1].?, succ.key);
237-
self.len += 1;
279+
self.len = old_len;
238280
} else {
239281
self.merge(node, index);
282+
const old_len = self.len;
240283
_ = self.deleteFromNode(node.children[index].?, key);
241-
self.len += 1;
284+
self.len = old_len;
242285
}
243286
}
244287

src/ordered/cartesian_tree.zig

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -220,36 +220,36 @@ pub fn CartesianTree(comptime K: type, comptime V: type) type {
220220
stack: std.ArrayList(*Node),
221221
allocator: Allocator,
222222

223-
pub fn init(allocator: Allocator, root: ?*Node) Iterator {
223+
pub fn init(allocator: Allocator, root: ?*Node) !Iterator {
224224
var it = Iterator{
225-
.stack = .{},
225+
.stack = std.ArrayList(*Node){},
226226
.allocator = allocator,
227227
};
228-
it.pushLeft(root);
228+
try it.pushLeft(root);
229229
return it;
230230
}
231231

232232
pub fn deinit(self: *Iterator) void {
233233
self.stack.deinit(self.allocator);
234234
}
235235

236-
fn pushLeft(self: *Iterator, node: ?*Node) void {
236+
fn pushLeft(self: *Iterator, node: ?*Node) !void {
237237
var current = node;
238238
while (current) |n| {
239-
self.stack.append(self.allocator, n) catch return; // Handle potential allocation failure
239+
try self.stack.append(self.allocator, n);
240240
current = n.left;
241241
}
242242
}
243243

244244
// src/cartesian_tree.zig
245245

246-
pub fn next(self: *Iterator) ?struct { key: K, value: V } {
246+
pub fn next(self: *Iterator) !?struct { key: K, value: V } {
247247
// self.stack.pop() returns `?*Node`.
248248
// The `if` statement correctly handles the optional, unwrapping it into `node`.
249249
if (self.stack.pop()) |node| {
250250
// 'node' is now a valid `*Node` pointer.
251251
if (node.right) |right_node| {
252-
self.pushLeft(right_node);
252+
try self.pushLeft(right_node);
253253
}
254254
return .{ .key = node.key, .value = node.value };
255255
} else {
@@ -259,7 +259,7 @@ pub fn CartesianTree(comptime K: type, comptime V: type) type {
259259
};
260260

261261
/// Create iterator for in-order traversal
262-
pub fn iterator(self: *const Self, allocator: Allocator) Iterator {
262+
pub fn iterator(self: *const Self, allocator: Allocator) !Iterator {
263263
return Iterator.init(allocator, self.root);
264264
}
265265
};
@@ -424,14 +424,14 @@ test "CartesianTree: iterator traversal" {
424424
try tree.putWithPriority(20, 20, 20);
425425
try tree.putWithPriority(5, 5, 5);
426426

427-
var iter = tree.iterator(testing.allocator);
427+
var iter = try tree.iterator(testing.allocator);
428428
defer iter.deinit();
429429

430430
// Should iterate in sorted key order (BST property)
431431
const expected = [_]i32{ 5, 10, 20, 30 };
432432
var idx: usize = 0;
433433

434-
while (iter.next()) |entry| : (idx += 1) {
434+
while (try iter.next()) |entry| : (idx += 1) {
435435
try testing.expectEqual(expected[idx], entry.key);
436436
}
437437
try testing.expectEqual(@as(usize, 4), idx);

src/ordered/sorted_set.zig

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ pub fn SortedSet(
1616

1717
pub fn init(allocator: std.mem.Allocator) Self {
1818
return .{
19-
.items = .{},
19+
.items = std.ArrayList(T){},
2020
.allocator = allocator,
2121
};
2222
}
@@ -29,10 +29,16 @@ pub fn SortedSet(
2929
return compare(key, item);
3030
}
3131

32-
/// Adds a value to the vector, maintaining sort order.
33-
pub fn add(self: *Self, value: T) !void {
32+
/// Adds a value to the set, maintaining sort order.
33+
/// Returns true if the value was added, false if it already existed.
34+
pub fn add(self: *Self, value: T) !bool {
3435
const index = std.sort.lowerBound(T, self.items.items, value, compareFn);
36+
// Check if value already exists
37+
if (index < self.items.items.len and compare(self.items.items[index], value) == .eq) {
38+
return false;
39+
}
3540
try self.items.insert(self.allocator, index, value);
41+
return true;
3642
}
3743

3844
/// Removes an element at a given index.
@@ -61,9 +67,9 @@ test "SortedSet basic functionality" {
6167
var vec = SortedSet(i32, i32Compare).init(allocator);
6268
defer vec.deinit();
6369

64-
try vec.add(100);
65-
try vec.add(50);
66-
try vec.add(75);
70+
_ = try vec.add(100);
71+
_ = try vec.add(50);
72+
_ = try vec.add(75);
6773

6874
try std.testing.expectEqualSlices(i32, &.{ 50, 75, 100 }, vec.items.items);
6975
try std.testing.expect(vec.contains(75));
@@ -89,7 +95,7 @@ test "SortedSet: single element" {
8995
var vec = SortedSet(i32, i32Compare).init(allocator);
9096
defer vec.deinit();
9197

92-
try vec.add(42);
98+
_ = try vec.add(42);
9399
try std.testing.expect(vec.contains(42));
94100
try std.testing.expectEqual(@as(usize, 1), vec.items.items.len);
95101

@@ -98,28 +104,31 @@ test "SortedSet: single element" {
98104
try std.testing.expectEqual(@as(usize, 0), vec.items.items.len);
99105
}
100106

101-
test "SortedSet: duplicate values" {
107+
test "SortedSet: duplicate values rejected" {
102108
const allocator = std.testing.allocator;
103109
var vec = SortedSet(i32, i32Compare).init(allocator);
104110
defer vec.deinit();
105111

106-
try vec.add(10);
107-
try vec.add(10);
108-
try vec.add(10);
112+
const added1 = try vec.add(10);
113+
const added2 = try vec.add(10);
114+
const added3 = try vec.add(10);
109115

110-
// Duplicates are allowed in this implementation
111-
try std.testing.expectEqual(@as(usize, 3), vec.items.items.len);
116+
// Duplicates should be rejected in a proper Set
117+
try std.testing.expect(added1);
118+
try std.testing.expect(!added2);
119+
try std.testing.expect(!added3);
120+
try std.testing.expectEqual(@as(usize, 1), vec.items.items.len);
112121
}
113122

114123
test "SortedSet: negative numbers" {
115124
const allocator = std.testing.allocator;
116125
var vec = SortedSet(i32, i32Compare).init(allocator);
117126
defer vec.deinit();
118127

119-
try vec.add(-5);
120-
try vec.add(-10);
121-
try vec.add(0);
122-
try vec.add(5);
128+
_ = try vec.add(-5);
129+
_ = try vec.add(-10);
130+
_ = try vec.add(0);
131+
_ = try vec.add(5);
123132

124133
try std.testing.expectEqualSlices(i32, &.{ -10, -5, 0, 5 }, vec.items.items);
125134
}
@@ -132,7 +141,7 @@ test "SortedSet: large dataset" {
132141
// Insert in reverse order
133142
var i: i32 = 100;
134143
while (i >= 0) : (i -= 1) {
135-
try vec.add(i);
144+
_ = try vec.add(i);
136145
}
137146

138147
// Verify sorted
@@ -147,11 +156,11 @@ test "SortedSet: remove boundary cases" {
147156
var vec = SortedSet(i32, i32Compare).init(allocator);
148157
defer vec.deinit();
149158

150-
try vec.add(1);
151-
try vec.add(2);
152-
try vec.add(3);
153-
try vec.add(4);
154-
try vec.add(5);
159+
_ = try vec.add(1);
160+
_ = try vec.add(2);
161+
_ = try vec.add(3);
162+
_ = try vec.add(4);
163+
_ = try vec.add(5);
155164

156165
// Remove first
157166
_ = vec.remove(0);

0 commit comments

Comments
 (0)