Improve MutableLinkedList for 1.0 #873
Conversation
| function Base.delete!(l::MutableLinkedList, idx) | ||
| Expr(:meta, :noinline) | ||
| Base.depwarn("`delete!(l::MutableLinkedList,idx::Int)` is deprecated, use `deleteat!(l,idx)` instead.", :delete!) | ||
| deleteat!(l,idx) | ||
| end | ||
|
|
||
| function Base.delete!(l::MutableLinkedList, r::UnitRange) | ||
| Expr(:meta, :noinline) | ||
| Base.depwarn("`delete!(l::MutableLinkedList,r::UnitRange)` is deprecated, use `deleteat!(l,idx)` instead.", :delete!) | ||
| deleteat!(l,r) | ||
| end |
There was a problem hiding this comment.
Why can't this be done with the @deprecate macro?
|
|
||
| Base.iterate(l::MutableLinkedList) = l.len == 0 ? nothing : (l.node.next.data, l.node.next.next) | ||
| Base.iterate(l::MutableLinkedList, n::ListNode) = n === l.node ? nothing : (n.data, n.next) | ||
|
|
||
| Base.isempty(l::MutableLinkedList) = l.len == 0 | ||
| Base.length(l::MutableLinkedList) = l.len | ||
| Base.collect(l::MutableLinkedList{T}) where T = T[x for x in l] | ||
| Base.eltype(::Type{<:MutableLinkedList{T}}) where T = T | ||
| Base.collect(l::MutableLinkedList{T}) where {T} = T[x for x in l] |
There was a problem hiding this comment.
Do we need this?
I would have thought it would fall back to this if we didn't define it.
| length(l1) == length(l2) || return false | ||
| for (i, j) in zip(l1, l2) | ||
| i == j || return false | ||
| end | ||
| return true | ||
| end | ||
|
|
||
| function Base.map(f::Base.Callable, l::MutableLinkedList{T}) where T | ||
| function Base.map(f::Base.Callable, l::MutableLinkedList{T}) where {T} |
There was a problem hiding this comment.
It would be good to strip out a lot of these constraints that do not follow modern practices.
| function Base.map(f::Base.Callable, l::MutableLinkedList{T}) where {T} | |
| function Base.map(f, l::MutableLinkedList{T}) where {T} |
| @@ -89,162 +113,227 @@ function Base.map(f::Base.Callable, l::MutableLinkedList{T}) where T | |||
| end | |||
| end | |||
|
|
|||
| function Base.filter(f::Function, l::MutableLinkedList{T}) where T | |||
| function Base.filter(f::Function, l::MutableLinkedList{T}) where {T} | |||
There was a problem hiding this comment.
| function Base.filter(f::Function, l::MutableLinkedList{T}) where {T} | |
| function Base.filter(f, l::MutableLinkedList{T}) where {T} |
Thanks for the review. As soon as I've time I'll look into your recommendations. |
It started as a simple inspection into MutableLinkedList and ended into a thorough refactoring of the code. As a consequence bugs have been fixed, and the API is now in alignment with Base.
_insert,_remove!,_traverse)@boundscheck: traversal (=indexing) is O(n), so the@boundscheckare not going to be a big deal.show()is now limited to the space given and has the same apperance asVectorVector{T}()append!(l,collections...)filter!(f,l)reverse!(l)empty!(l)splice!(l,idx)splice!(l,range)there is another PR for splice! but I found that it was flawedMutableLinkedList(iter)without a type parameter infers the eltype fromiterlastandfirstreturn a BoundsError if the list is empty instead of anArgumentErrorto mirrorVectorpopat!(l,idx)throws aBoundsErrorif the list is emtpy instead of anArgumentErrorto mirrorVectordelete!has been deprecated todeleteat!deleteappend!()will not mutate the last elementFixes
MutableLinkedList's #794Supersedes:
In case this gets merge it needs to be squashed, the single commits aren't useful on their own.