Feedback from implementing iterators

[jacobdufault]

I've been working on writing an iterator that is relatively boiler-plate free, akin to how slices can do for (slice) |value| { ... } and for (slice) |value, idx| { ... }. I was pleasantly surprised with how well everything composes in general, but a bit of feedback on how it could be nicer:

Here is some code for a custom iterator. Notes at bottom.

const Iterator = struct {
 count: i32,
 current: i32,

 pub fn next(self: &Iterator) ?i32 {
 if (self.current >= self.count) return null;
 const result = self.current;
 self.current += 1;
 return result;
 }

 const WithIndex = struct {
 value: i32,
 idx: i32
 };
 pub fn nextWithIndex(self: &Iterator) ?WithIndex {
 // It'd be much more natural to write this with an early return.
 if (self.next()) |value| {
 return WithIndex {
 .value = value,
 .idx = 1
 };
 }

 return null;
 }
};

fn buildIterator(count: i32) Iterator {
 return Iterator {
 .count = count,
 .current = 0
 };
}

fn useCase() void {
 { var it = buildIterator(10); while (it.next()) |i| {
 std.debug.warn("i={}\n", i);
 } }
 // It'd be great if `|e|` could be `|value, index|`
 { var it = buildIterator(10); while (it.nextWithIndex()) |e| {
 std.debug.warn("value={}, idx={}\n", e.value, e.idx);
 } }
}

1. In nextWithIndex, an early return would read more cleanly, especially when code starts getting complicated and there are a lot of reasons to exit the function. However, the unwrapping if makes this hard to write ; maybe if can adjust the type for the rest of the block so it is no longer optional?

2. The while loops in useCase are very ugly to look at (the double } }) and require a lot more typing than the typical for (init; cond; increment) loop.

3. Instead of using a proxy type WithIndex, it'd be great if the |e| could unwrap directly to |value, idx|. Two quick ideas on how this could be done:

   • multiple return values
   • simple name-based pattern matching

Another option is to standardize this type of thing into the compiler so for (foo) |x| looks for a method with a specific name/result-type, ie, for (foo) |x| ==> next1() IteratorResult(u32), for (foo) |x, y| ==> next2() IteratorResult(u32, bool), etc.

[andrewrk]

Thanks for the feedback. I can help you with one thing real quick, and I'll get back to the language change suggestions later:

 pub fn nextWithIndex(self: &Iterator) ?WithIndex {
 return WithIndex {
 .value = self.next() ?? return null,
 .idx = 1
 };
 }

One thing I will say, is that Zig has been happy to trade aesthetic syntactic beauty in exchange for language simplicity and conformity. So while I agree with you that

{
 var it = buildIterator(10);
 while (it.next()) |i| {
 std.debug.warn("i={}\n", i);
 }
}

looks more verbose than C's for loop, it's a tough sell to add a not strictly necessary language feature, only for the sake of beauty.

It's pretty harmless to drop the outer curly braces, since it's consistent with almost all variable declarations that we don't try to end the scope when we stop using them:

var it = buildIterator(10);
while (it.next()) |i| {
 std.debug.warn("i={}\n", i);
}

That doesn't look so bad to me.

I'll respond to your third point later. Thanks again for the feedback!

[thejoshwolfe]

Here's another minor tip, subject to your preference:

 pub fn next(self: &Iterator) ?i32 {
 if (self.current >= self.count) return null;
 defer {self.current += 1;}
 return self.current;
 }