-1

Hello I just want to know what is wrong in my code.

This is a problem in the book 'Think Python' which asks to write a function to return True is the list has any duplicate elements, or False otherwise.

def has_duplicates(t):
    for i in t:
        if i in t.pop(t.index(i)):
            return True
    return False
teamathematic
  • 3
  • 1

5 Answers5

3

What's wrong with it?

  • You remove elements from t while iterating over it. This prevents the iteration working as you'd expect, generally the effect is to skip elements. Do not do this.

  • t.pop(t.index(i)) returns i (or a value equal to it), so whatever you're hoping to achieve by if i in, I don't think you will achieve.

Steve Jessop
  • 273,490
  • 39
  • 460
  • 699
  • This is not an answer, this is a comment. Plese delete it and post a comment instead. – webzy Jul 26 '15 at 09:42
  • @webzy "Hello I just want to know what is wrong in my code." I'd say this is an answer. – juanchopanza Jul 26 '15 at 09:45
  • 1
    I've taken the view here that although the headline question is in effect "what is the solution to the exercise I'm doing?", this does not constrain me to solve the exercise for the questioner. I'm considering whether or not to solve the exercise (for performance this breaks in three cases: contents are hashable, contents are less-than-comparable but not hashable, contents are only equality-comparable), but it depends how the questioner responds :-) – Steve Jessop Jul 26 '15 at 09:49
  • This exactly tells me "what is wrong with my code" thanks Steve Jessop – teamathematic Jul 26 '15 at 09:52
  • 1
    But the title of your question was: "How to see if there are any duplicate elements in list in Python" ;-) – webzy Jul 26 '15 at 09:53
1

You can test it by comparing the length of the list to the length of the set created from that list, because set removes duplicates.

def has_duplicates(t):
    return len(t) > len(set(t))
webzy
  • 249
  • 2
  • 7
  • Since the questioner is constrained not to use `set` (which for someone reason is left until a comment to mention), just for fun let's suppose the elements of the list are not hashable. For example, perhaps they are also lists :-) – Steve Jessop Jul 26 '15 at 09:42
  • @Amit Talmor Ha ha! It's not about winning, it's about helping ;-) – webzy Jul 26 '15 at 09:45
  • @webzy - at least I didn't forget to add a closing parenthesis to len(set(t)...;-) – Amit Talmor Jul 26 '15 at 09:50
0
def has_duplicates(t):
    no_dup = list(set(t))
    if len(t) > len(no_dup):
       return True
    return False
Amit Talmor
  • 7,174
  • 4
  • 25
  • 29
-1

First of all, never tamper with a list while iterating on it.. Second, your code is checking if i == the item you're popping. That will always be true since when you're popping an item python returns you that item. So it's no use to compare it with i, because you have just popped i. It's like comparing if i == i.. Try your function for t = ['1', '2', '3'] and use a debugger to verify what I'm saying..

You could do the following since you can't use sets:

def has_duplicates(t):
    #first have a backup to keep the iteration clean
    t_backup = t.copy()
    for i in t_backup:
        #first pop the item
        t.pop(t.index(i))
        #then check if such item still exists
        if i in t:
            return True
    return False
John Paraskevopoulos
  • 969
  • 10
  • 24
-2

Well, with list comprehension (thanks to Steve Jessop):

def has_duplicates(t):
    return any(t.count(i)>1 for i in t)

lst = [1, 2, 3, 4, 5, 6, 1, 5 ,6]
print(has_duplicates(lst))

result:

>>> 
True
>>>
Clodion
  • 1,017
  • 6
  • 12
  • This returns `False` for input `[2,2]`, because `range(len(t))` should just be `t`. `True if something else False` can be simplified to `bool(something)`, although in this case `something` is already a boolean so just `something`. Also, you could get some benefit of short-circuiting by doing `any(t.count(i) > 1 for i in t)`. – Steve Jessop Jul 26 '15 at 09:54
  • Yes, sure! I look at your comments! Thanks! – Clodion Jul 26 '15 at 09:57
  • 1
    By putting in `[]`, you prevent `any` from short-circuiting, since now it's required that the whole list of true/false values is constructed before `any` is called. – Steve Jessop Jul 26 '15 at 15:26
  • Ho yes! Thanks again! – Clodion Jul 26 '15 at 16:14