-1

So there's a problem in leetcode that's pretty simple but the solution is incorrect for the second: (OG question: Given an array nums, write a function to move all 0's to the end of it while maintaining the relative order of the non-zero elements.

For example, given nums = [0, 1, 0, 3, 12], after calling your function, nums should be [1, 3, 12, 0, 0].

Note: You must do this in-place without making a copy of the array. Minimize the total number of operations. )

class Solution(object):
def moveZeroes(self, nums):
    """
    :type nums: List[int]
    :rtype: void Do not return anything, modify nums in-place instead.
    """
    k = 0
    for ele in nums[:]:
        if ele == 0:
            nums.remove(0)
            k += 1
    nums.extend([0]*k)


class Solution(object):   -------Incorrect solution
def moveZeroes(self, nums):
    """
    :type nums: List[int]
    :rtype: void Do not return anything, modify nums in-place instead.
    """
    k = 0
    for ele in nums:
        if ele == 0:
            nums.remove(0)
            k += 1
    nums.extend([0]*k)

Why does that make a difference please?

Edccccch
  • 19
  • 1
  • 3
  • 8
  • 5
    The first solution is also incorrect, because it makes a copy of the input. – user2357112 Aug 09 '16 at 20:07
  • 2
    http://stackoverflow.com/questions/6022764/python-removing-list-element-while-iterating-over-list <-- See here. In the correct solution, you are iterating over a _copy_ of the list, in the incorrect solution, you iterate over the list itself. – Will Aug 09 '16 at 20:08
  • 2
    Check my answer here to see why it's bad to remove things form a sequence you are currently iterating over: http://stackoverflow.com/a/31704332/1318181 – kylieCatt Aug 09 '16 at 20:10
  • 1
    In addition, you should double-check your method's indentation. – albert Aug 09 '16 at 20:14
  • Wee-woo! The PEP 8 police have fined you for not conforming with PEP 8: reason: using camelBack. You should use `name_with_underscores` – noɥʇʎԀʎzɐɹƆ Aug 09 '16 at 23:31

2 Answers2

0

You can both not copy the list and not modify the list while it's being looped over:

class Solution(object):
    def moveZeroes(self, numbers):
        """
        :type numbers: List[int]
        :rtype: void Do not return anything, modify numbers in-place instead.
        """
        k = 0

        while True:
            try:
                numbers.remove(0)
                k += 1
            except ValueError:
                break

        numbers.extend([0] * k)

numbers = [0, 1, 0, 3, 12]

print(id(numbers), "->", numbers)

Solution().moveZeroes(numbers)

print(id(numbers), "->", numbers)

OUTPUT:

(4348993904, '->', [0, 1, 0, 3, 12])
(4348993904, '->', [1, 3, 12, 0, 0])
cdlane
  • 40,441
  • 5
  • 32
  • 81
0

You can't modify a list while looping over it. It's illegal in Python.

[:] makes an independent copy of the list to loop over.

But the question requires you to not make a copy of the list, so both solutions are incorrect.

I won't tell you exactly how to fix it because I want you to learn, but you should create a set. Whenever you would use .remove(), add it to the set. Check to see if an element is in the set before starting your for loop. At the end of the loop, actually remove the elements in the set from the list.

To create a set, use set(). To add an element to a set, use set_name.add()


Don't use leetcode to learn Python. Leetcode is for technical coding interviews, and your python isn't very good. You wouldn't pass the interview.

Your code violates PEP 8 and your class doesn't inherit off of object. You also have your indentation done wrong, which results in an error in Python. Get an editor like PyCharm (it's free) that can warn you of these errors in the future. It's a great IDE, seriously, try it out.

noɥʇʎԀʎzɐɹƆ
  • 9,967
  • 2
  • 50
  • 67