3
\$\begingroup\$

I'm not sure if this code is industry-standard or not. Please review it and let me know if any improvement is required.

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


class getLinkRest():
       head = None
       tail= None

   def insert(self,data):  
      node=Node(data,None)
     if self.head is None:
       self.head=self.tail=node
     else:
       self.tail.next=node
     self.tail=node


 def showFromItem(self,data):
     curr_node=self.head
     flag=None
     while curr_node is not None:
          if curr_node.data == data or curr_node.data > data:
            print curr_node.data
            flag =1
            curr_node=curr_node.next
    if(flag is None):
       print "Not exist"

  s= getLinkRest()
  s.insert(1)
  s.insert(2)
  s.insert(3)
  s.insert(4)
  s.insert(5)
  s.showFromItem(3)
  s.showFromItem(7)
\$\endgroup\$
0
7
\$\begingroup\$

Fix the indentation

Before I say anything, let's reformat your code, for everyone's sake, because as you posted it, it just doesn't work, because indentation is crucial in Python. Please be more careful next time.

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


class getLinkRest():
    head = None
    tail = None

    def insert(self, data):
        node = Node(data, None)

        if self.head is None:
            self.head = self.tail = node
        else:
            self.tail.next = node
        self.tail = node

    def showFromItem(self, data):
        curr_node = self.head
        flag = None
        while curr_node is not None:
            if curr_node.data == data or curr_node.data > data:
                print curr_node.data
                flag = 1
            curr_node = curr_node.next

        if flag is None:
            print "Not exist"

s = getLinkRest()
s.insert(1)
s.insert(2)
s.insert(3)
s.insert(4)
s.insert(5)
s.showFromItem(3)
s.showFromItem(7)

Purpose of the code

Finding the kth to last element of a singly linked list

This is not what the posted code does. The showFromItem function searches for an element with the specified value, and prints that element and the rest of the elements until the end of the linked list that are bigger than the the specified value, or else it prints the text "Not exist". For example:

s.insert(11)
s.insert(12)
s.insert(13)
s.insert(4)
s.insert(15)
s.showFromItem(13)

The above code will print 13 and 15.

That's a very unusual logic, and somehow I doubt that this was really your intention.

It's good to have a clear purpose, and to include a clear problem statement in a question.

Unused constructor argument

The next value of the constructor is never used. So you could simplify it:

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

Instead of using flag with None and 1 as values, it would be more natural to make it a boolean, like this:

    found = False
    while curr_node is not None:
        if curr_node.data == data or curr_node.data > data:
            print(curr_node.data)
            found = True
        curr_node = curr_node.next

    if not found:
        print("Not exist")

You can write this condition shorter:

if curr_node.data == data or curr_node.data > data:

Like this:

if curr_node.data >= data:
\$\endgroup\$
8
  • \$\begingroup\$ getLinkRest is a bad non-PEP8 class name, isn't it? \$\endgroup\$ – Eric Jul 26 '15 at 21:10
  • \$\begingroup\$ thanks @janos for your valuable feedback. I am trying to add few items ,suppose 1-5, first will add into singly linked list and then will provide a number for example 3 ..So it will show starting from 3 to 5 ..I also wanted to show a message if we will not find a random number which does not belongs to that linked list. I am trying to learn python as well as linked list part ...I am following 'cracking the code' book as a reference ... \$\endgroup\$ – saikat123 Jul 26 '15 at 21:16
  • \$\begingroup\$ @Eric yes it is. Ethan has already mentioned PEP8, that's I didn't mention it in my review. \$\endgroup\$ – janos Jul 26 '15 at 21:21
  • \$\begingroup\$ @saikat123 I believe you refer to the book Cracking the coding interview? I too have this book, but the description you gave is not related to the question that is posed in the book. The idea is: if you have a list 2 3 4 5 1 6 and if k=3, then you should return the third last item, which would be 5. \$\endgroup\$ – DJanssens Jul 27 '15 at 17:28
  • \$\begingroup\$ @DJanssens You are right ... it is not working when I changed list and gave random value .. let me modify code..Initially my target was I will create a linked list which may consist from 1-5 and then I will provide any number starting from 1 to 5 .it will display from that number up to end. As per that logic, it is working. \$\endgroup\$ – saikat123 Jul 28 '15 at 2:53
5
\$\begingroup\$

Style

Python has an official style guide, PEP8. You have many style violations in your code. Here are a few of those violations.

  1. You should have whitespace around comparison, mathematical, binary, assignment, and comma operators.
  2. Anywhere where your code should be indented should be indented by four spaces, or one tab with a width of of four spaces.
  3. You don't need parentheses around conditions in if statements. For example, this: if(flag is None): should become this: if flag is None:.
  4. Functions and variables should be in snake_case, classes should be in PascalCase, and constant variables (unchanging values) should be in UPPER_SNAKE_CASE.
  5. Your naming is not so great. There are many not so great names, but here are a few that'd I'd change.

    1. next -> next_item_index
    2. curr_node -> current_node
    3. data -> value
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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