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?
1 Answer 1
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.
self.head
norself.tail
is ever updated anymore. \$\endgroup\$