trjhtr

This is a follow up question from here. I have revised my code as
per the suggestions of peer reviewers. I still feel there is lot of
scope for improvement in this code.

class Node(object):
 def __init__(self,data, next=None):
 self.data = data
 self.next = next

class LinkedList(object):

 def __init__(self):
 self.head = None
 self.size =0


 def extend(self, seq = None):
 """extends list with the given sequence"""

 for i in range(0, len(seq)):
 node = Node(seq[i])
 self.size +=1
 node.next = self.head
 self.head = node

 def append(self, item):
 """append item to the end of list"""
 node = Node(item)
 node.next = self.head
 self.head = node
 self.size += 1

 def printdata(self):
 """print elements of linked list"""
 node = self.head 
 while node:
 print node.data
 node = node.next

 def __iter__(self):
 node = self.head
 while node:
 yield node.data
 node = node.next

 def __contains__(self, item):
 """checks whether given item in list"""
 node = self.head 
 while node:
 if node.data == item:
 return True
 node = node.next


 def len(self):
 """returns the length of list"""
 return self.size


 def remove(self, item):
 """removes item from list"""
 node = self.head
 current = self.head.next
 if node.data == item:
 self.size -= 1
 self.head = current
 current = current.next
 while current:
 if current.data == item:
 current = current.next
 node.next = current
 self.size -= 1
 node = current
 current = current.next




 def __str__(self):
 return str(self.data) + str(self.size)

test cases :

if __name__ == "__main__":

 llist = LinkedList()

 llist.extend([98,52,45,19,37,22,1,66,943,415,21,785,12,698,26,36,18,
 97,0,63,25,85,24])

 print "Length of linked list is ", llist.len()

 llist.append(222)

 print "Length of linked list is ", llist.len()

 llist.remove(22)

 print "Elements of linked list n", llist.printdata()
 print "Length of linked list is ", llist.len()

 ## Search for an element in list 
 while True:
 item = int(raw_input("Enter a number to search for: "))
 if item in llist:
 print "It's in there!"
 else:
 print "Sorry, don't have that one"

• 
  

  The construct

  
  
  

  for i in range(0, len(seq)):
   node = Node(seq[i])
  

  
  
  

  is usually frowned upon. Consider a Pythonic

  
  
  

  for item in seq:
   node = Node(item)
  

  
  



• I see no reason to default seq to None.




• append does not append. It prepends.



• 
  

  remove has an unpleasant code duplication. To avoid special casing the head, consider using a dummy node:

  
  
  

  def remove(self, item):
   dummy = Node(None, self.head)
   prev = dummy
   while prev.next:
   if prev.next.data == item:
   prev.next = prev.next.next
   size -= 1
   prev = prev.next
   self.head = dummy.next
  

  
  


• 
  

  printdata may (or shall?) use the iterator:

  
  
  

  def printdata(self):
   for node in self:
   print node.data
  

  
  

I suggest the following:

• Remove the default argument from extend. There is no upside to allowing client code to call a.extend() with no argument, since it's an error anyway.


• 
  

  Use the existing append function to simplify extend as follows:

  
  
  

  def extend(self, x):
for item in x:
self.append(item)

  
  


• 
  

  Use the iterator you have written wherever possible. Don't forget about the built-in function any. For example,

  
  
  

  def __contains__(self, a):
return any(a == item for item in self)

  
  


• 
  

  In the iterator it would be better to change the while statement slightly (since None is the specific value that terminates the iteration):

  
  
  

  def __iter__(self):
node = self.head
while node is not None:
yield node.data
node = node.next

  
  


• 
  

  Since you never call the Node constructor with more than one argument, I would remove the second argument. I would also rename the class _Node so that client code is discouraged from messing with it (it exists solely to be a helper class for the linked list).

  
  
  

  class _Node(object):
def __init__(self, data):
self.data = data
self.next = None