1
\$\begingroup\$

Below is my attempt at a doubly-linked list in Python (3):

class LinkedListNode:
 def __init__(self, value):
 self.value = value
 self.left = None
 self.right = None
class LinkedList:
 def __init__(self):
 self.head = self.tail = None
 self.size = 0
 def get_first(self):
 if self.head:
 return self.head.value
 raise Exception('Reading from an empty list.')
 def get_last(self):
 if self.tail:
 return self.tail.value
 raise Exception('Reading from an empty list.')
 def add_first(self, value):
 node = LinkedListNode(value)
 if self.head:
 node.right = self.head
 self.head.left = node
 self.size += 1
 else:
 self.head = self.tail = LinkedListNode(value)
 self.size = 1
 def add_last(self, value):
 node = LinkedListNode(value)
 if self.tail:
 node.left = self.tail
 self.tail.right = node
 self.size += 1
 else:
 self.head = self.tail = node
 self.size = 1
 def remove_first(self):
 if self.head:
 ret = self.head.value
 self.head = self.head.right
 self.head.left = None
 self.size -= 1
 return ret
 raise Exception('Removing from an empty list.')
 def remove_last(self):
 if self.tail:
 ret = self.tail.value
 self.tail = self.tail.left
 self.tail.right = None
 self.size -= 1
 return ret
 raise Exception('Removing from an empty list.')
 def __repr__(self):
 str = '['
 if self.size > 0:
 str += self.head.value
 node = self.head.right
 while node:
 str += ', ' + node.value
 node = node.right
 return str + ']'
def main():
 list = LinkedList()
 while True:
 cmd_line = input()
 if cmd_line == 'quit':
 break
 command_tokens = cmd_line.split()
 command = command_tokens[0]
 try:
 if command == "get_first":
 print(list.get_first())
 elif command == "get_last":
 print(list.get_last())
 elif command == "add_first":
 list.add_first(command_tokens[1])
 elif command == "add_last":
 list.add_last(command_tokens[1])
 elif command == "remove_first":
 list.remove_first()
 elif command == "remove_last":
 list.remove_last()
 elif command == "print":
 print(list)
 except Exception:
 print("Unknown command")
main()

How would I rewrite it in a professional manner?

Mast
13.8k12 gold badges57 silver badges127 bronze badges
asked Jan 14, 2019 at 6:28
\$\endgroup\$
2
  • \$\begingroup\$ Are you sure this is a version you want to review? It has gaping bugs - once the list has a node, neither self.head nor self.tail is ever updated anymore. \$\endgroup\$ Commented Jan 14, 2019 at 21:25
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers . \$\endgroup\$ Commented Jan 15, 2019 at 14:33

1 Answer 1

3
\$\begingroup\$

A better main

Instead of just calling main() at the bottom, wrap it in if __name__ == "__main__":. In otherwords:

if __name__ == "__main__":
 main()

This allows me to re-use your module better, see the answer I linked for more details.

if chains

This portion of code:

 if command == "get_first":
 print(list.get_first())
 elif command == "get_last":
 print(list.get_last())
 elif command == "add_first":
 list.add_first(command_tokens[1])
 elif command == "add_last":
 list.add_last(command_tokens[1])
 elif command == "remove_first":
 list.remove_first()
 elif command == "remove_last":
 list.remove_last()
 elif command == "print":
 print(list)

Is a code smell to me. I would refactor this with a dictionary:

commands = {
 "get_first": lambda: print(list.get_first()),
 "get_last": lambda: prrint(list.get_last()),
 ...
}

This can then be called with:

commands[command]()

Library Shadowing

list = LinkedList()

list is already part of the standard library. I would not over shadow the name. I would change the name to something like linked for instance.

namedtuple?

It is questionable whether you want to do this or not, but the class LinkedListNode is just an __init__ method. In this case, I would usually advocate using a namedtuple instead and using the defaults parameter added in Python 3.7 if applicable. Although, then again, defaults is only in 3.7... So, maybe not applicable. Still useful to know about.

Implement __str__ how you implemented __repr__

__str__ is for human-readable output. __repr__ is for more internal information. You have implemented __repr__ to display a human readable representation of your list. Switch to __str__ instead.

answered Jan 14, 2019 at 7:56
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.