patternpythonMinor
Skip list in Python
Viewed 0 times
pythonlistskip
Problem
I've made an attempt at writing a skip list in Python. I'm using NumPy to generate geometric random variables, but since that's a bit of a heavy dependency to drag around I could easily implement that myself.
I based my implementation on the basic (none of the improvements such as capping node height etc.) algorithm here.
What do you think needs improvement?
```
import numpy as np
class SkipList:
def __init__(self, p=0.5):
"""
Create a Skiplist object.
>>>> l = Skiplist() # An empty skip list
>>>> l = Skiplist.from_iter(zip(range(5), range(5))) # A skip list from an iterable
"""
self.p = p
self.head = SkipList.Node()
self.max_height = 1
self.__length = 0
def from_iter(it, p=0.5):
"""
Create a SkipList from an iterable of (Key, Value) tuples
"""
s = SkipList(p=p)
for k, v in it:
s.insert(k, v)
return s
def __getitem__(self, key):
curr = self.head
for level in range(self.max_height - 1, -1, -1):
while curr.forward[level] and curr.forward[level].key self.max_height:
new_forward += [None for _ in range(self.max_height, height)]
self.head.forward += [None for _ in range(self.max_height, height)]
update += [self.head for l in range(self.max_height, height)]
self.max_height = height
new_node = SkipList.Node(key=key, value=value, forward=new_forward)
for l, n in enumerate(update[:height]):
n.forward[l] = new_node
self.__length += 1
def __delitem__(self, key):
curr = self.head
update = [None for _ in range(self.max_height)]
for level in range(self.max_height - 1, -1, -1):
while curr.forward[level] and curr.forward[level].key < key:
curr = curr.forward[level]
update[level] = c
I based my implementation on the basic (none of the improvements such as capping node height etc.) algorithm here.
What do you think needs improvement?
```
import numpy as np
class SkipList:
def __init__(self, p=0.5):
"""
Create a Skiplist object.
>>>> l = Skiplist() # An empty skip list
>>>> l = Skiplist.from_iter(zip(range(5), range(5))) # A skip list from an iterable
"""
self.p = p
self.head = SkipList.Node()
self.max_height = 1
self.__length = 0
def from_iter(it, p=0.5):
"""
Create a SkipList from an iterable of (Key, Value) tuples
"""
s = SkipList(p=p)
for k, v in it:
s.insert(k, v)
return s
def __getitem__(self, key):
curr = self.head
for level in range(self.max_height - 1, -1, -1):
while curr.forward[level] and curr.forward[level].key self.max_height:
new_forward += [None for _ in range(self.max_height, height)]
self.head.forward += [None for _ in range(self.max_height, height)]
update += [self.head for l in range(self.max_height, height)]
self.max_height = height
new_node = SkipList.Node(key=key, value=value, forward=new_forward)
for l, n in enumerate(update[:height]):
n.forward[l] = new_node
self.__length += 1
def __delitem__(self, key):
curr = self.head
update = [None for _ in range(self.max_height)]
for level in range(self.max_height - 1, -1, -1):
while curr.forward[level] and curr.forward[level].key < key:
curr = curr.forward[level]
update[level] = c
Solution
Remove repetition
You have almost identical code:
You may avoid the repetition writing:
The nested loops:
Are repeated identical 3 times, extract them into a function.
Use the
You do not need a manual for loop in
You may also use
It makes the code even nearer to English (The length should be equal and all pairs should be equal)
You have almost identical code:
def items(self):
"""
Generator in the style of dict.items
"""
curr = self.head.forward[0]
while curr:
yield (curr.key, curr.value)
curr = curr.forward[0]
def __iter__(self):
curr = self.head.forward[0]
while curr:
yield curr.key
curr = curr.forward[0]You may avoid the repetition writing:
def __iter__(self):
for key, _ in self.items():
yield keyThe nested loops:
for level in range(self.max_height - 1, -1, -1):
while curr.forward[level] and curr.forward[level].key < key:
curr = curr.forward[level]Are repeated identical 3 times, extract them into a function.
Use the
all built-inYou do not need a manual for loop in
__eq__:def __eq__(self, other):
if len(self) != len(other):
return False
return all(self_pair == other_pair
for self_pair, other_pair in zip(self.items(), other.items())all and avoiding tuple unpacking is closer to how you would describe the function in English (all pairs should be equal)You may also use
and instead of a separate ifdef __eq__(self, other):
return len(self) == len(other) and \
all(self_pair == other_pair
for self_pair, other_pair in zip(self.items(), other.items())It makes the code even nearer to English (The length should be equal and all pairs should be equal)
Code Snippets
def items(self):
"""
Generator in the style of dict.items
"""
curr = self.head.forward[0]
while curr:
yield (curr.key, curr.value)
curr = curr.forward[0]
def __iter__(self):
curr = self.head.forward[0]
while curr:
yield curr.key
curr = curr.forward[0]def __iter__(self):
for key, _ in self.items():
yield keyfor level in range(self.max_height - 1, -1, -1):
while curr.forward[level] and curr.forward[level].key < key:
curr = curr.forward[level]def __eq__(self, other):
if len(self) != len(other):
return False
return all(self_pair == other_pair
for self_pair, other_pair in zip(self.items(), other.items())def __eq__(self, other):
return len(self) == len(other) and \
all(self_pair == other_pair
for self_pair, other_pair in zip(self.items(), other.items())Context
StackExchange Code Review Q#118340, answer score: 2
Revisions (0)
No revisions yet.