Adds back generic typesafe simple single and doubly linked lists #23590
Conversation
|
If we do this, then type-safe version should delegate to type-erased one, instead of being a reimplementation. See https://github.com/tigerbeetle/tigerbeetle/blob/main/src/stack.zig for an example. No strong opinion on whether std should or should not include such a type-safe wrapper, but I'd personally lean towards no, in the interesting of simplicity. |
|
I think if you read the commentary here: https://news.ycombinator.com/item?id=43679707 it's clear that @fieldParentPtr is sufficiently confusing and not being typesafe is also a magnet for (justifiable) criticism. A typesafe impl could serve to help "teach" readers of the code:
An API that is "closer to what people expect from a linked list is good" but I agree that delegation would be better. I couldn't figure out how to square the circle on the first pass but I have some ideas and may take another swipe at it later today. |
|
thanks @matklad it was way easier than I thought, I don't know why I made things more complicated the first time around. |
ityonemo
force-pushed
the
adds-generic-typesafe-simple-LL
branch
from
38a9f6b to
79f7da3
Compare
| // Iterate over all nodes, returning the count. | ||
| /// | ||
| /// This operation is O(N). Consider tracking the length separately rather than | ||
| /// computing it. | ||
| pub fn len(list: SimpleLinkedList) usize { | ||
| return list.wrapped.len(); | ||
| } |
There was a problem hiding this comment.
Would it not be better to just not have .len() to discourage use of such a function?
When using a linked-list this is not an operation one should really ever use (same goes for the intrusive linked-list) as it touches all nodes thus involves several cache-line misses in the worst case. It may be O(N) but the cost is not comparable to say the sum of an array which is also O(N) but likely involves fewer cache misses.
I'd say the same for at as well. Those should be rare or another structure should be chosen in most cases.
There was a problem hiding this comment.
well, if that's the case it then shouldn't be in base linked list implementation, either. I think it's fine. For debugging/asserts, if you know your lists are not that long, etc.
ityonemo
changed the title
ityonemo
changed the title
just something to consider. If it's not merged, I don't mind, but I think having a typesafe (simple) option will prevent people from footgunning themselves. Also could serve as a reference for someone who wants to build their own typesafe versions for more complex situations.