Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I'm new to Python and thus learning it by implementing a set of commonly used data structures. Here's my implementation of a LinkedList. First the Node class.

class Node:

    def __init__(self, value = None, next = None):
        self.value = value;
        self.next = next;

Here's the LinkedList,

from node import Node

class LinkedList:

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

    def insert(self, item):
        curr = Node();
        curr.value = item
        self.size += 1
        if(self.head == None):
            self.head = curr
        else:
            curr.next = self.head
            self.head = curr

    def remove(self, item):
        if(self.head == None):
             raise Exception('Removing from an empty list')
        curr = self.head
        if(curr.value == item):
            self.head = curr.next
            return
        while(curr.next.value != item):
            curr = curr.next
        if(curr.next == None):
            raise Exception('Item not present in list')
        else:
            curr.next = curr.next.next



    def print_list(self):
        curr = self.head;
        while(curr):
            print(curr.value, end=' ')
            curr = curr.next

And finally the calling client,

from linked_list import LinkedList

list = LinkedList()
list.insert(2)
list.insert(5)
list.insert(7)
list.insert(12)
list.insert(13)
list.remove(13)
list.print_list()

My primary language is Java, so I'd invite comments on how pythonic is my implementation and best practices. TIA.

share|improve this question
up vote 2 down vote accepted

For the most part it looks good, I haven't tested performance, but nothing stands out as bad. You should probably rewrite remove, so that it takes an index rather than a value. Imagine

list = LinkedList()
list.insert(2)
list.insert(2)
list.remove(2)

Your current code works great as long as there aren't duplicate values, but once there are everything gets kind of messed up.

With regards to pythonicness, the only comment I have is that if(x): is harder to type and harder to parse than if x:.

share|improve this answer

You should not name variables and parameters the same as Python built-in names, such as next (see this).

The reason is because you are redefining what Python already defined. Now anyone, including you, relying on that Python builtin by importing your code might see unexpected side effects. See this simple illustration; once you modify Python builtin list to be an instance of list, one cannot call list() again:

>>> x = list()
>>> x
[]
>>> list = [5]
>>> y = list()
Traceback (most recent call last):
  File "<input>", line 1, in <module>
TypeError: 'int' object is not callable
share|improve this answer
    
Why ? Please state the reason as to why this is not a good idea. I'm sure it's a valid critic to the code, but we're here to help OP get better :). – Marc-Andre Feb 18 at 18:22

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.