Doubly-linked list implementation in Python with unit-tests

I am soliciting some feedback on my doubly-linked list implementation. In particular, I am looking for your take on my unit-tests and their coverage of the code itself. That is, I developed this class using the test-driven development approach and am wondering how it turned out. In addition, I would welcome any suggestions on making the code more readable and/or efficient (eliminate redundancies, etc).

```
class DLinkedList(object):
_size = 0
class Node(object):
def __init__(self, _value, _previous, _next):
self.value = _value
self.next = _next
self.previous = _previous

def __eq__(self, other):
return (self.value == other.value) and (self.next == other.next) and (self.previous == other.previous)

def __init__(self):
self.head = None
self.tail = None
self._size = 0

def __len__(self):
if self.head:
return self._size
else:
return 0

def __str__(self):
current = self.head
to_print = []
while current:
to_print.append(current.value)
current = current.next
return str(to_print)

def insert(self, _value, _position=0, _previous = None, _next = None):
if _value is None:
raise ValueError('Cannot add None item to a list')
if _position < 0:
raise ValueError('Cannot add to negative position in the list')
if _position == 0:
if self.head:
new_node = self.Node(_value, None, self.head)
old_node = self.head
self.head = new_node
old_node.previous = new_node
self._size = self._size + 1
else:
new_node = self.Node(_value, None, None)
self.tail = new_node
self.head = new_node
self._size = self._size + 1
return new_node
else:
curr

Here are some high-level "round 1" notes to make the code cleaner and more Pythonic:

• since you are implementing a "doubly linked list", name the class DoublyLinkedList instead of a more questionable DLinkedList

-
I don't see much point in nesting the Node class inside the linked list, define it separately (and, you know, "Flat is better than nested"):

class Node(object):
    # ...

class DoublyLinkedList(object):
    # ...

This would also lead to using Node in place of self.Node (and you would also need to use node variable name in place of Node in the insert() method)

-
there is not need to define _size on the class level - leave it defined in the class constructor:

class DoublyLinkedList(object):
    _size = 0  # <-- REMOVE THIS LINE

-
do not put underscore prefix in front of method arguments names - this kind of notation is reserved for "private" attributes (not enforced by the language though)

-
I would also remove the _ from the _size variable - I don't see why would you "hide" it from your linked list's public API

-
you can also use the size += 1 and size -= 1 shortcuts instead of size = size + 1 and size = size - 1

-
you can omit the parenthesis around the if conditions. For instance, you may replace:

def __eq__(self, other):
    return (self.value == other.value) and (self.next == other.next) and (self.previous == other.previous)

with:

def __eq__(self, other):
    return self.value == other.value and \
           self.next == other.next and \ 
           self.previous == other.previous

Some other thoughts:

• check out attrs package that is quite handy and helps to avoid a lot of classes/properties/attributes boilerplate

• ideally, the tests should give a clear idea on how the code under test is used, give a good sense of the available API and it's usage. The tests you currently have are not quite readable, try to make the test methods smaller aiming to "one assertion per test method" rule (which itself is extreme, but good to keep in mind), making more descriptive test method names

-
what if we improve the linked list "pretty-printing" and, instead of calling str() on a list, we'll use ` separator. Something like:

def __str__(self):
    current = self.head

    to_print = []
    while current:
        to_print.append(current.value)
        current = current.next

    return "(HEAD) {items} (TAIL)".format(items="  ".join(map(str, to_print)))

would result into this printed linked list version:

In [1]: dll = DoublyLinkedList()

In [2]: dll.insert(1, 0)
   ...: dll.insert(3, 0)
   ...: dll.insert(5, 0)

In [3]: print(dll)
(HEAD) 5  3  1 (TAIL)

-
I think there is a separation of concerns issue. It feels like the Node instead of the linked list itself should decide where the value passed to it's constructor is an allowed one. Consider moving the value "is None" check to the Node` class.

StackExchange Code Review Q#158047, answer score: 2