Exercise 48: Bad optional style?
chancyk opened this issue · 9 comments
https://github.com/ratfactor/ziglings/blob/main/exercises/048_methods2.zig#L13
// Elephant tail methods!
pub fn getTail(self: *Elephant) *Elephant {
return self.tail.?; // Remember, this means "orelse unreachable"
}
pub fn hasTail(self: *Elephant) bool {
return (self.tail != null);
}Does this not defeat the point of optional types? In that if I forget to call hasTail, the compiler can't help me catch that I've forgotten self.tail can be null since we've overridden the optional behavior using .??
Do you mean something like this
if (e.tail) |tail| {
e = tail;
} else {
break;
}
would be better?
@chrboesch I think so. The getTail implementation panics at runtime versus the compiler catching the error if you were to just e = e.tail without the check in your example.
Using e = e.getTail();:
C:\Projects\demos\zigdemos\ziglings\exercises (main -> origin)
λ zig run 048_methods2.zig
A B C thread 18468 panic: attempt to use null value
C:/Projects/demos/zigdemos/ziglings/exercises/048_methods2.zig:14:9: 0x7ff7d7962266 in getTail (048_methods2.exe.obj)
return self.tail.?; // Remember, this means "orelse unreachable"
^
C:/Projects/demos/zigdemos/ziglings/exercises/048_methods2.zig:56:22: 0x7ff7d7961878 in visitElephants (048_methods2.exe.obj)
e = e.getTail();
^
C:/Projects/demos/zigdemos/ziglings/exercises/048_methods2.zig:41:19: 0x7ff7d7961355 in main (048_methods2.exe.obj)
visitElephants(&elephantA);
^
C:\Bin\zig-windows-x86_64-0.10.0\lib\std\start.zig:377:41: 0x7ff7d7961297 in WinStartup (048_methods2.exe.obj)
std.debug.maybeEnableSegfaultHandler();
^
???:?:?: 0x7ffd93fb26bc in ??? (???)
???:?:?: 0x7ffd94fadfb7 in ??? (???)
Using e = e.tail;:
C:\Projects\demos\zigdemos\ziglings\exercises (main -> origin)
λ zig run 048_methods2.zig
048_methods2.zig:56:14: error: expected type '*048_methods2.Elephant', found '?*048_methods2.Elephant'
e = e.tail;
~^~~~~
048_methods2.zig:56:14: note: cannot convert optional to payload type
048_methods2.zig:56:14: note: consider using `.?`, `orelse`, or `if`
048_methods2.zig:56:14: note: '?*048_methods2.Elephant' could have null values which are illegal in type '*048_methods2.Elephant'
referenced by:
main: 048_methods2.zig:41:5
callMain: C:\Bin\zig-windows-x86_64-0.10.0\lib\std\start.zig:596:17
remaining reference traces hidden; use '-freference-trace' to see all reference traces
In this case, my suggestion would look like this:
// New Elephant methods!
pub fn getTail(self: *Elephant) ?*Elephant {
return self.tail; // Remember, this means "orelse unreachable"
}
if (e.getTail()) |tail| {
e = tail;
} else {
break;
}
I withdraw the proposal because, while technically correct, it distorts the meaning of the methods. There is still something missing in the considerations.
I think you could stay align with the previous optional2 exercise with your previous example:
pub fn getTail(self: *Elephant) ?*Elephant {
return self.tail;
}
But reduce the while loop by :
while (true) {
e.print();
e.visit();
e = e.getTail() orelse break; // Which method do we want here?
}
It's just an another suggestion...
And also it can be align with the quiz6.
A nice suggestion, but it won't work. But this would:
while (true) {
e.print();
e.visit();
// This gets the next elephant or stops.
e = if (e.hasTail()) e.??? else break; // Which method do we want here?
}Sry miss type e = e.tail orelse break become e = e.getTail() orelse break.
and so the hasTail is no more used.
It build successfully.
Did i miss something ?
When I try e = e.getTail() orelse break I get error: expected optional type, found.
But before we start looking for a smoother solution, let us keep in mind what is the exercise for: "methods".
We should try not to eliminate all methods for a nicer look. Of course, there are better solutions, but then can we still convey the theme?
I think it will not get much better. But if someone has a better solution, we can open this issue again.