Skip to main content
3 of 5
from list of findings towards review
greybeard
  • 6.6k
  • 3
  • 20
  • 52

Strategy
To support (coding and) code review (especially when asking for alternatives), state (in the code) the goal pursued during coding. (Double linkage/last serves no purpose I can see.)
If not literally re-inventing a support on solid surfaces combining low longitudinal friction with good lateral guide, use/implement an existing "protocol"/"interface" - mutable sequence comes to mind seeing the methods you present; something like test_MutableSequence() may fall into your lap.
Explicitly specify everything protocol but not standard using docstrings. Sketch tests: If you don't know what to test, you don't know what to implement.

Tactics:
- specify what is to happen with parameter values without "natural" meaning, e.g., index smaller zero or not smaller count.
- (as @Peilonrayz demanded:) Don’t Repeat Yourself
- provide docstrings for classes (and modules), too
- check comments and, arguably more important, docstrings for correctness - when a second error passes unit testing after thinking unit tests complete, revise unit tests

Observations about the code, [especially] insert, find, replace, at_index:
- insert fails to update count
- find, replace: replace (& _find) might use find. With quality satisfied with more than one node's data, both are underspecified.
- _at_index: if count/2 < index < count, walk backwards
(it may be useful to allow indices from -count (even with at_index or generally) - cf. slicing)
- delete looks non-adapted from a singly-linked list implementation (no need for last)
current.next.last should be set depending on current == tail
should use find()
- insert fails to set old_head.last
- reverse: how about transmuting to an instance of ListLinkedDoubly, with roles of next and last exchanged(&head/tail, if sticking with no sentinel node (@Peilonrayz, again))
As presented, reverse() is a costly operation - reversed() returns an iterator
- __str__: return '[]' if 0 == self.count \
    else '[-(' + ')<=>('.join(self.items()) + ')-]'

Trying to stay DRY implementing pre/append():

def _add(self, item):
    """Update count and return new Node,
    attached to list if empty before.
    """
    self.count += 1
    new_node = Node(item)

    if self.head is None:
        self.head = self.tail = new_node
    return new_node

def append(self, item):
    """Return self with item inserted at the tail."""
    new_node = self._add(item)

    if 1 < self.count:
        self.tail.next = new_node
        new_node.last = self.tail
        self.tail = new_node

def prepend(self, item):
    """Return self with item inserted at the head."""
    new_node = self._add(item)

    if 1 < self.count:
        self.head.last = new_node
        new_node.next = self.head
        self.head = new_node

(I "real life", I like to be able to chain calls: return self)

greybeard
  • 6.6k
  • 3
  • 20
  • 52