1
\$\begingroup\$

I am writing the linked list data structure, using this question's requirements (the method set).

I have written two working methods, which do the similar thing and differs by only few lines. So, I tried to refactor them and although I move the duplicate part to the separate function, the code became more verbose. Also, it added two function calls overhead, that is not good for data structure implementations.

The question:

  1. Do you see another way to duplication elimination?
  2. I understand, that in this particular case, it will be better to leave everything as it is, without refactoring. But from the point of best production practices and "pythonic" way, which variant will be better?

Before refactoring

def insert_before_key(self, key, value):
        new_node = Node(value)
        prev, curr = self._find_key(key)

        if curr:
            if prev:
                new_node.next = curr            # differs
                prev.next = new_node            # differs
            else:
                new_node.next = self.head       # differs
                self.head = new_node            # differs

def insert_after_key(self, key, value):
        new_node = Node(value)
        prev, curr = self._find_key(key)

        if curr:
            if prev:
                new_node.next = curr.next       # differs
                curr.next = new_node            # differs
            else:
                new_node.next = self.head.next  # differs
                self.head.next = new_node       # differs

After refactoring

# Common part was moved to this function
def insert(self, key, value, with_prev, non_prev):
    new_node = Node(value)
    prev, curr = self._find_key(key)

    if curr:
        if prev:
            with_prev(prev, curr, new_node)
        else:
            non_prev(prev, curr, new_node)

# put two little chunks of code to the nested functions 
# and pass them to the "insert" function, that implement the logic
def insert_before_key(self, key, value):
    def with_prev(prev, curr, new_node):
        new_node.next = curr
        prev.next = new_node

    def non_prev(prev, curr, new_node):
        new_node.next = self.head
        self.head = new_node

    self.insert(key, value, with_prev, non_prev)

def insert_after_key(self, key, value):
    def with_prev(prev, curr, new_node):
        new_node.next = curr.next
        curr.next = new_node

    def non_prev(prev, curr, new_node):
        new_node.next = self.head.next
        self.head.next = new_node

    self.insert(key, value, with_prev, non_prev)
\$\endgroup\$
  • 2
    \$\begingroup\$ This is roughly what I would have done if I had thought trying to reduce duplication was worth it here. I'm not sure it is worth it here though. I find the generalized code harder to read, and the original functions are small enough that the gain isn't huge. \$\endgroup\$ – Carcigenicate Mar 22 at 20:04

Your Answer

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

Browse other questions tagged or ask your own question.