How do I set the prev of my first node to the next of my last node to create a circular doubly linked list?

364 Views Asked by At

I am creating the Josephus problem using a circular doubly linked list. I am getting an Attribute error, which I assume is because my current_node (first node) does not have a .prev yet.

I understand that the prev of my first node should point to the next of my last node to create a circular doubly linked list.

Can someone guide me on whether I have correctly identified the error? If yes, how can I rectify it?

If not, then what are the other ways I can correct the error?

#Initialize the node
    def __init__(self, data):
        self.data = data
        self.next = None
        self.prev = None
        
    def remove(self, n):
        print("Student " +str(n)+ " was removed")
        
class Circle:
# Initializing the DLL
    def __init__(self):
        self.head = None
        self.tail = None
#Inserting elements 2 to n in the dll              
    def insert_after(self, x, data):          
        y = Student(data) # make a new Node object.
        z = Student(data)
        z = x.next
        
        y.prev = x
        y.next = z
        x.next = y
        
        z.prev = y       
    
    def josephus_solution(self, dllist, n, k):
            no_of_active_nodes = n
            current_node = Student(1)
            #last_node = Student(n)
            #print(current_node.prev)
            for i in range(2, n + 1):
                dllist.insert_after(current_node, i)
                count = 0
            #print(current_node.data)
            while (current_node.next != current_node.prev):
                #print(current_node.next.prev)
                current_node = current_node.next
                count += 1
                #print(current_node.data)
                if (count == k):
                    current_node.remove(current_node.data)
                    current_node.prev.next = current_node.next
                    current_node.next.prev = current_node.prev
                    count = 0 
                    no_of_active_nodes -= 1
                    #print(no_of_active_nodes)
                if (no_of_active_nodes == 1):
                    print("Student" + str(current_node.data) + "Recieves the scholarship")
            return current_node.data

dllist = Circle()
n = 5 #int(input('Input number of people (n): '))
k = 2 #int(input('The nth person will be executed. Input k: '))
ans = dllist.josephus_solution(dllist, n, k)

Error

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
/tmp/ipykernel_24/3762059582.py in <module>
     54 n = 5 #int(input('Input number of people (n): '))
     55 k = 2 #int(input('The nth person will be executed. Input k: '))
---> 56 ans = dllist.josephus_solution(dllist, n, k)

/tmp/ipykernel_24/3762059582.py in josephus_solution(self, dllist, n, k)
     32             #print(current_node.prev)
     33             for i in range(2, n + 1):
---> 34                 dllist.insert_after(current_node, i)
     35                 count = 0
     36             #print(current_node.data)

/tmp/ipykernel_24/3762059582.py in insert_after(self, x, data)
     24         x.next = y
     25 
---> 26         z.prev = y
     27 
     28     def josephus_solution(self, dllist, n, k):

AttributeError: 'NoneType' object has no attribute 'prev'
1

There are 1 best solutions below

0
trincot On

The direct reason for the error is that z is None and your code can therefore not access any prev attribute of it in the line z.prev = y. The cause is that when the first node is created, its prev and next attributes are initialised to None, and when this node is passed as x argument to insert_after, then with z = x.next a None value is assigned to z.

There are several issues with your approach:

  • In insert_after it makes no sense to call Student twice, since you want to insert one node, not two
  • Although your Circle class has a head and a tail attribute, they are never used after their initialisation to None, so there is actually nothing in the Circle instance that helps you to maintain the list. You might as well define all methods on the Student class.
  • The while condition in josephus_solution is probably intended to test that only one node remains in the list, but it actually verifies whether there are two left. Admittedly, this works when current_node is a node that was just deleted, but then the returned data is the data of the deleted node and not the remaining active node, and current_node is not a just-deleted node, then this condition will make the loop exit when it still has two active nodes.

Some other remarks:

  • As you call josephus_solution as a method, it should not be necessary to also pass an instance as argument. self is already available to that method
  • I would split the creation of the nodes into a separate -- generic -- method, and let josephus_solution work on an existing linked list.
  • As in a circular list none of the next or prev attributes should ever be None, you'll make things easier when initialising them to self instead of None.
  • The remove method should not be responsible for printing a message, but should on the other hand be responsible for rewiring the prev and next references. At any rate, there is no need for this method to get an argument, as it knows its own attributes through self.
  • In the original Josephus problem, the first node is numbered as 1, so with k=2 the first node to remove is the one with number 2. Your code would first delete the one with number 3. To align with the original problem, move the current_node = current_node.next statement to be the last line in the body of the loop. This also helps to be sure that current_node is never a just-deleted node when the while condition is evaluated.
  • As your while condition already takes care of when to stop, there should be no reason to keep track with a no_of_active_nodes variable.

There are more remarks to make, but these are what I believe the most important.

Here is a correction of your code, taking all of the above into account:

class Student:
    def __init__(self, data):
        self.data = data
        # Better make prev/next self references to avoid None:
        self.next = self
        self.prev = self
        
    def remove(self):
        # Perform the remove here
        self.prev.next = self.next
        self.next.prev = self.prev
        
    def insert(self, data):          
        # Should only create one node, not two
        node = Student(data)
        node.next = self
        node.prev = self.prev
        self.prev = node.prev.next = node

    # Handy method to create a circular list from values
    @classmethod
    def fromiterator(cls, iterator):
        head = cls(next(iterator))
        for value in iterator:
            head.insert(value)
        return head

    def josephus_solution(self, k):
        count = 0
        current_node = self
        while current_node.next is not current_node:  # This is how to test for 1 node
            count += 1
            if count == k:
                current_node.remove()  # No argument
                print("Student " +str(current_node.data) + " was removed")
                count = 0
            current_node = current_node.next # do this at the end of the iteration
            # No need to keep track of active nodes ...
        return current_node

# These are the parameters of the "original" problem (see Wikipedia)
n = 41
k = 3
dllist = Student.fromiterator(iter(range(1, n+1)))
# Don't pass the list as argument: it already is available as `self`
dllist = dllist.josephus_solution(k)
# Perform output in the main code, not inside method
print("Student " + str(dllist.data) + " recieves the scholarship")