5

I am implementing a LinkedList in python(3.7.4) and the code of the module is below :-

LinkedList.py

class Node:
    def __init__(self,value):
        self.value = value
        self.ref = None

class LinkedList(Node):
    def __init__(self):
        self.__head = None
        self.__cur = None
        self.__count = 0

    def add(self,value):
        if self.__head is None:
            self.__cur = Node(value)
            self.__head = self.__cur
        else:
            self.__cur.ref = Node(value)
            self.__cur = self.__cur.ref
        self.__count += 1

    def getList(self):
        temp = self.__head
        while temp!=None:
            yield temp.value
            temp = temp.ref

    def delete(self,value):
        temp = self.__head
        while temp!=None:
            if temp.value == value and temp == self.__head:
                self.__head = temp.ref
                del temp
                self.__count -= 1
                break
            elif temp.ref != None and temp.ref.value == value:
                temp_ref = temp.ref.ref
                del temp.ref
                self.__count -= 1
                temp.ref = temp_ref
                break
            temp = temp.ref

    def __getitem__(self,index):
        i = 0
        temp = self.__head

        if type(index) is int:
            while temp!=None:
                if i == index:
                    return temp.value
                temp = temp.ref
                i += 1

        elif type(index) is slice:
            if index.start is None:
                start = 0
            else:   start = index.start

            if index.stop is None:
                stop = self.__count
            else:   stop = index.stop

            if index.step is None:
                step = 1
            else:   step = index.step

            returningList = list()
            while temp!=None:
                if start <= i < stop:
                    returningList.append(temp.value)

                if i==0:
                    i = start
                    for _ in range(start):
                        if temp != None:
                            temp = temp.ref
                else:
                    i+=step
                    for _ in range(step):
                        if temp != None:
                            temp = temp.ref
            return returningList

    def __len__(self):
        return self.__count

All the above functions are working well, there is no any error in this module.

but my problem is __getitem__() method. I am unable to make the exact logic for that and it is going too larger.

also it is not working for negative indices like obj[-1] returning me nothing ( len(obj) is not 0 here).

can anyone give or suggest me proper logic for __getitem__() method for code optimization and complexity reduction.

Er. Harsh Rathore
  • 758
  • 1
  • 7
  • 21

3 Answers3

2

You can do this, for example:

def __getitem__(self, index):
    if isinstance(index, int):
        if index < 0:
            index = len(self) + index
        # check if `index` is valid
        # search for the element as you're currently doing.
    elif isinstance(index, slice):
        return [self[i] for i in range(len(self))[index]]
    else:
        raise ValueError(f'Linked list cannot be indexed with values of type {type(index)}')

UPDATE: the code above is very concise, but it's also tremendously slow. If I'm not mistaken, it's a bit better than O(n**2), while the code below is at least 71.58 times faster (doing linkedListWith500Elements[::-1]), and it should be about O(n)!

This should be way faster because it doesn't iterate through the list each time to retrieve the next element of the slice:

class LinkedList:
    ...

    def __iter__(self):
        temp = self.__head
        while temp is not None:
            yield temp.value
            temp = temp.ref

    def __getitem__(self, index):
        if isinstance(index, int):
            if index < 0:
                index = len(self) + index

            for i, value in enumerate(self):
                if i == index:
                    return value
            raise IndexError(f'{type(self).__name__} index {index} out of range(0, {len(self)})')
        elif isinstance(index, slice):
            rangeOfIndices = range(len(self))[index]
            isRangeIncreasing = rangeOfIndices.start <= rangeOfIndices.stop + 1 and rangeOfIndices.step > 0


            rangeOfIndices = iter(rangeOfIndices) if isRangeIncreasing else reversed(rangeOfIndices)

            retval = []  # you can preallocate this...
            updateRetval = retval.append if isRangeIncreasing else (lambda value: retval.insert(0, value))  # ...and change this accordingly, although I haven't tested whether it'll be faster

            try:
                searchingForIndex = next(rangeOfIndices)
            except StopIteration:
                return retval

            temp = self.__head   
            for i, element in enumerate(self):
                if temp is None:
                    break

                if i == searchingForIndex:
                    updateRetval(temp.value)

                    try:
                        searchingForIndex = next(rangeOfIndices)
                    except StopIteration:
                        return retval

                temp = temp.ref

            return retval
        raise ValueError(f'{type(self).__name__} can only be indexed with integers or slices (not {type(index)})')

Preallocating the list should be around 22% faster:

...
rangeOfIndices = range(len(self))[index]
isRangeIncreasing = rangeOfIndices.start <= rangeOfIndices.stop + 1 and rangeOfIndices.step > 0

# preallocate the list...     
retval = [None] * len(rangeOfIndices)   

if isRangeIncreasing:
    retvalIndex = 0
    rangeOfIndices = iter(rangeOfIndices)
    # ...and use a different update function
    def updateRetval(value):
        nonlocal retvalIndex
        retval[retvalIndex] = value
        retvalIndex += 1
else:
    retvalIndex = len(retval) - 1
    rangeOfIndices = reversed(rangeOfIndices)
    def updateRetval(value):
        nonlocal retvalIndex
        retval[retvalIndex] = value
        retvalIndex -= 1

try:
...
ForceBru
  • 43,482
  • 10
  • 63
  • 98
  • okay, But what about my logic or size of code. Is it exact or can I further optimize it? – Er. Harsh Rathore Aug 07 '19 at 18:46
  • This doesn't work with slices: `TypeError: 'slice' object cannot be interpreted as an integer` – Wombatz Aug 07 '19 at 18:50
  • @ErHarshRathore, I think the code for when `isinstance(index, int)` is `True` is fine, but the code for the other case seems to large, provided that you can reuse the `__getitem__` method. – ForceBru Aug 07 '19 at 18:54
  • Still crashed when any of the slice parts is `None`. `linked_list[1:4]` -> `slice(1, 4, None)` – Wombatz Aug 07 '19 at 18:55
  • @Wombatz, okay, does the current version work? Looks like all the edge cases have now been accounted for. – ForceBru Aug 07 '19 at 18:58
  • Hey! what about recursion I can call recursive function for every int value of slice – Er. Harsh Rathore Aug 07 '19 at 19:08
  • @ErHarshRathore, yep, that's exactly what my code's doing – ForceBru Aug 07 '19 at 19:10
  • Apart from the SyntaxError, this crashes when the `slice.stop` is larger then the length of the list and `linked_list[::-1]` returns an empty list. – Wombatz Aug 07 '19 at 19:12
  • there is again a problem with complexity in recursion I have to start tracing always from the head of list for each int value of slice whether is 50 or 500000. – Er. Harsh Rathore Aug 07 '19 at 19:20
  • `[self[i] for i in range(len(self))[index]]` seems like it will be very inefficient. – juanpa.arrivillaga Aug 07 '19 at 20:08
  • @ErHarshRathore, I've updated my answer, it should be much faster now – ForceBru Aug 07 '19 at 21:49
  • Thanks for your support. You helped me a lot and from this answer I got some key-points. I am upvoting you but not accepting this answer yet because the length of code is too large yet also it has been failed with positive to negative slicing with positive step-size like `x=iter(range(len(obj))[3,-1,1])` it should return list from 3rd upto last element. – Er. Harsh Rathore Aug 07 '19 at 22:49
  • I want to implement each and every probability for `[start,stop,step]` values with small size of code and at most I have done with your help.I will post my solution as soon as possible. So thanks again☺☺ – Er. Harsh Rathore Aug 07 '19 at 22:58
  • @ErHarshRathore, "it should return list from 3rd upto last element" - and it does. The last element won't be included, though: `[1,2,3,4,5,6,7,8,9,0][3:-1:1] == [4, 5, 6, 7, 8, 9]`. As for the size of the code: sometimes even huge amounts of code are faster than, say, a simple single line. That's how loop unrolling and function inlining works, for example. So, small code isn't necessarily the fastest. – ForceBru Aug 08 '19 at 06:45
1

To solve this with the least amount of code, you can first convert your linked list into a python list and then slice that python list.

But first you have to rename your method getList(self) to __iter__(self). Thats the canonical name anyways.

Now __getitem__ becomes one line:

def __getitem__(self, index):
    return list(self)[index]

This is not very space efficient, since it duplicates your list.

Edit:

Here is a more efficient solution. I assume again that getList(self) is renamed to __iter__.

def __getitem__(self, index):
    # creates all requested indices or just one int
    indices = range(len(self))[index]  # constant time and space

    if isinstance(indices, int):
        return next(value for i, value in enumerate(self) if i == indices)  # takes time proportional to the index but constant space
    else:
        # the double reversing is needed if the step is negative
        reverse = -1 if index.step < 0 else 1  # constant time
        return [value for i,value in enumerate(self) if i in indices[::reverse]][::reverse]  # O(n) time and O(len(indices)) space

This is efficient, because testing if an int is in a range and slicing a range takes constant time.

Wombatz
  • 4,958
  • 1
  • 26
  • 35
0

I have implemented a recursive function to solve this problem with an extra functional parameter which will show the position of current cursor to save time in over and over start tracing from first node each and every time when recursion called.

I am not saying that other answers are wrong but I used to implement principle of KISS in my code and other answers were hard to understand.

My edited code for __getitem__() is below:

class LinkedList(Node):
    ...

    def __getitem__(self,index,fromNode=None):
        global i,temp
        if fromNode is None:
            i,temp = 0,self.__head

        if isinstance(index, int):
            if index<0: index+=len(self) 
            while temp!=None:
                if i == index:
                    return temp.value
                temp = temp.ref
                i += 1

        elif isinstance(index, slice):
            step = 1    if index.step is None else index.step
            start = 0   if index.start is None else index.start
            stop=len(self) if index.stop is None else index.stop

            if start < 0:   start += len(self)
            elif step> 0 >stop: stop += len(self)

            reverseFlag = True if step<0 else False

            if reverseFlag:
                trace = iter(reversed(range(len(self))[start:stop:step]))
            else:
                trace = iter(range(len(self))[start:stop:step])

            returningList = [self.__getitem__(indice,temp) for indice in trace]

            return list(reversed(returningList)) if reverseFlag else returningList

    ...

Can I further reduce this code or can reduce more complexity from it? If yes then please suggest me via your answer or comments.

Er. Harsh Rathore
  • 758
  • 1
  • 7
  • 21