Python Iterator class index overflows on second of for loop run

Question

I defined a class that holds a list collection of objects, and defined the __iter__ and __next__ methods to make it for loopable. The collection here, is a Deck class that holds a list of Card objects.

Code:

import random

class Card:

    @staticmethod
    def get_ranks():
        return ("A", "K", "Q", "J", "10", "9", "8", "7", "6", "5", "4", "3", "2") # A is highest, 2 is lowest

    @staticmethod
    def get_suites():
        return ("H", "D", "S", "C")

    def __init__(self, suite, rank):
        if suite not in Card.get_suites():
            raise Exception("Invalid suite")
        if rank not in Card.get_ranks():
            raise Exception("Invalid rank")
        self.suite = suite
        self.rank = rank

    def __lt__(self, card2):
        self_rank = Card.get_ranks().index(self.rank)
        card2_rank = Card.get_ranks().index(card2.rank)
        return self_rank > card2_rank

    def __le__(self, card2):
        self_rank = Card.get_ranks().index(self.rank)
        card2_rank = Card.get_ranks().index(card2.rank)
        return self_rank >= card2_rank

    def __gt__(self, card2):
        self_rank = Card.get_ranks().index(self.rank)
        card2_rank = Card.get_ranks().index(card2.rank)
        return self_rank < card2_rank

    def __ge__(self, card2):
        self_rank = Card.get_ranks().index(self.rank)
        card2_rank = Card.get_ranks().index(card2.rank)
        return self_rank <= card2_rank

    def __eq__(self, card2):
        self_rank = Card.get_ranks().index(self.rank)
        card2_rank = Card.get_ranks().index(card2.rank)
        return self_rank == card2_rank

    def __ne__(self, card2):
        self_rank = Card.get_ranks().index(self.rank)
        card2_rank = Card.get_ranks().index(card2.rank)
        return self_rank != card2_rank

    def __str__(self):
        return(self.rank + self.suite)

    def __repr__(self):
        return str(self)

class Deck:

    def __init__(self):
        self.contents = [Card(suite, rank) for suite in Card.get_suites() for rank in Card.get_ranks()]
        random.shuffle(self.contents)
        self.index = 0

    def __len__(self):
        return len(self.contents)

    def __iter__(self):
        return self

    def __next__(self):
        if self.index == len(self.contents):
            raise StopIteration
        item = self.contents[self.index]
        self.index += 1
        return item

    def pick_card(self):
        choice = random.randrange(len(self))
        card = self.contents.pop(choice)
        return card
    
    def return_card_and_shuffle(self, card):
        self.contents.append(card)
        random.shuffle(self.contents)

    def __str__(self):
        dstr = ''
        for card in self:
            dstr += str(card) + ", "
        return "{} cards: ".format(len(self)) + dstr[:-2]

def deal_bookends(deck):
        card1 = deck.pick_card()
        card2 = deck.pick_card()
        if card1 > card2:
            temp = card1
            card1 = card2
            card2 = temp
        return (card1, card2)

if __name__ == '__main__':
    deck = Deck()
    for _ in range(3):
        c1, c2 = deal_bookends(deck)
        print("We have {} and {}".format(c1, c2))
        print(deck)
        deck.return_card_and_shuffle(c1)
        print(deck)
        print(deck.contents[-4:])
        deck.return_card_and_shuffle(c2)
        print(deck)
        print(deck.contents[-4:])

On running, I get the following error:

We have 8H and KH
50 cards: 9H, 8C, AC, 7C, 6H, 2S, 2D, 5C, 10H, 5H, JS, 5S, KD, JH, JC, QS, 2H, 3H, 3S, 3D, 4C, 4H, AD, KS, JD, QH, 10D, 6S, 5D, 8D, 3C, 6C, 7D, AS, 7H, AH, 9S, 10C, QC, QD, 7S, 2C, KC, 8S, 4D, 4S, 6D, 10S, 9D, 9C
51 cards: QS
[7D, 5C, 10H, QS]
52 cards: 10C
[KC, 3S, 9H, 10C]
We have 2C and QD
Traceback (most recent call last):
  File "playing_cards.py", line 106, in <module>
    print(deck)
  File "playing_cards.py", line 88, in __str__
    for card in self:
  File "playing_cards.py", line 73, in __next__
    item = self.contents[self.index]
IndexError: list index out of range

It seems the thing doesn't for the second run of the for loop when I push the card object back into the list. How do I solve this while keeping the pop,push functionality.

Edit: The self.index is at 50 after the first call to print(). When the card is added back to list, index remains at 50, whereas the deck length is now 51 cards. So in the second (and third) call to print the last card is printed instead of the entire deck. Then subsequently error is raised.

I think I have read the documentation wrong here. My question is should I reset the index at the StopIteration bit. Is that the correct way to do this, or is the index supposed to reset on its own?

Why not just return iter(self.contents) in __iter__? No need to reinvent the wheel. — Bailey Parker
– Bailey Parker, Commented Jan 22, 2018 at 5:42
Actually even better just subclass list. This will give you all of the behavior you want, then you can add deck specific stuff on top. — Bailey Parker
– Bailey Parker, Commented Jan 22, 2018 at 5:43
As you can see, I am trying to learn how to make an iterable here. I am doing something wrong in the process. reinventing the when is the point of this exercise. I know this can be done in easier ways. — kchak
– kchak, Commented Jan 22, 2018 at 5:45
I didn't see that which is why I made that suggestion :) But in that case, I'd recommend returning a new DeckIterator object that maintains its own position. In the way you have it if you iterate over the Deck within an iteration of the deck you'll have issues. See: docs.python.org/3.3/reference/datamodel.html#object.__iter__ — Bailey Parker
– Bailey Parker, Commented Jan 22, 2018 at 5:50
So in this case, I should just have __iter__ return self.contents and not implement __next__? — kchak
– kchak, Commented Jan 22, 2018 at 5:59

Bailey Parker · Accepted Answer · 2018-01-22 06:23:20Z

Note: If you are trying to learn how iterators work by implementing your own, then the above advice holds. If you just want to make your Deck iterable, you can just do this in Deck:

def __iter__(self):
    return self.contents  # lists are already iterable

Even better, if you want your deck to behave like a list (iterating, indexing, slicing, removal), you can just extend list.

Learning how iterators work:

The problem you have here is you are conflating a collection with an iterator. A collection should hold a group of items. Your Deck is a collection. A collection is iterable, which means I can do for x in collection on it. When we do for x in collection, Python actually does for x in iter(collection), which turns the collection into an iterator.

You want your iterator and collection to be separate. If you collection was its own iterator, then you can only have one iterator over it at a time (itself). Also note that iterators should only be used once. By doing self.index = 0 in your __iter__, you are making your iterator (Deck) reusable.

Consider the following:

nums = [1, 2, 3]

for i in nums:
    for j in nums:
        print(i, j)

We expect this to return:

Note that each time the inner loop iterates over the whole collection. If nums was its own iterator, then we'd have some issues:

# Here internally nums sets the current index as 0
for i in nums:
   # Here internally nums sets the current index as 0 again
   for j in nums:
       print(i, j)

   # Once this inner loop finishes the current index is 4.
   # But that is also the index for the outer loop, so the
   # outer loop ends too

Unexpected output:

1 1
1 2
1 3

The solution is Deck.__iter__ should return a new object called DeckIterator, which keeps track of its own index. DeckIterator.__iter__ should return self (as required by the docs), but that is just a detail. By doing this you enable multiple iterations over the deck at once that work as expected.

So a minimal example of this would be:

class Deck:
    # ... snip ...

    def __iter__(self):
        return DeckIterator(self.contents)

class DeckIterator:
    def __init__(self, cards):
        self.index = 0
        self.cards = cards

    def __iter__(self):
        return self

    def __next__(self):
        if self.index >= len(self.cards):
            # We've gotten the end of the deck/list
            raise StopIteration

        item = self.cards[self.index]
        self.index += 1
        return item

Also, if you don't believe me about this list as its own iterator, here's a list that exhibits this bad behavior:

class BadList(list):
    def __iter__(self):
        self._current_index = 0
        return self

    def __next__(self):
        print(f'current index is {self._current_index}', end='')

        if self._current_index >= len(self):
            print(' which is the end, so ending iteration')
            raise StopIteration

        item = self[self._current_index]
        print(f' so returning value {item}')
        self._current_index += 1
        return item

# Using letters instead of numbers so difference between indices
# and items is more clear
letters = BadList('abc')

for i in letters:
    for j in letters:
        print(i, j)

Output from it:

current index is 0 so returning value "a"
current index is 0 so returning value "a"
a a
current index is 1 so returning value "b"
a b
current index is 2 so returning value "c"
a c
current index is 3 which is the end, so ending iteration
current index is 3 which is the end, so ending iteration

Stephen Rauch · Accepted Answer · 2018-01-22 05:42:12Z

0

Not sure how you got there, but you are beyond the length of your list. Suggest you compare for >= length of the list like:

def __next__(self):
    if self.index >= len(self.contents):
        raise StopIteration
    .....

answered Jan 22, 2018 at 5:42

Stephen Rauch♦

50.1k32 gold badges119 silver badges143 bronze badges

Comments

Abhijith Asokan · Accepted Answer · 2018-01-22 05:56:40Z

0

Make the following changes,

def __iter__(self):
    self.index = 0
    return self

So that each time __iter__ is called, index is reset. The reason your'e getting this error is, once you iterate through the deck, at the end of the iteration, self.index == len(self.contents).

The next time you iterate, the self.index should be reset to 0.

I made the above change and it worked for me.

edited Jan 22, 2018 at 5:56

answered Jan 22, 2018 at 5:44

Abhijith Asokan

1,87512 silver badges15 bronze badges

Comments

Blckknght · Accepted Answer · 2018-01-22 06:52:12Z

Your specific issue at the moment is caused by the check in your __next__ method not being general enough to detect all situations where you've iterated past the last value in self.contents. Since self.contents can change, you need to use a greater-than-or-equal test:

if self.index >= len(self.contents):

This will fix the current issue, but you'll still have other problems, since your Deck can only be iterated once. That's because you've implemented the iterator protocol, rather than the iterable protocol. These are easy to confuse, so don't feel bad if you don't understand the difference immediately.

An iterable is any object with an __iter__ method that returns an iterator. Some iterables return different iterators each time they're called, so you can iterate on them multiple times.

An iterator implements a __next__ method, which yields the next value or raises StopIteration. An iterator must also have an __iter__ method, which returns itself, which allows an iterator to be used wherever an iterable is expected, though it can only be iterated on once.

For your Deck, it probably makes sense to implement the iterable protocol, and return a separate iterator each time __iter__ is called. It's only rarely useful to implement your own iterator type, but if you want to test your knowledge of how the different protocols fit together, it can be interesting:

class Deck:
    def __init__(self):
        self.contents = [Card(suite, rank)
                         for suite in Card.get_suites()
                         for rank in Card.get_ranks()]
        random.shuffle(self.contents)
        # no index here

    def __iter__(self):
        return DeckIterator(self)

    # other methods, but no __next__

class DeckIterator:
    def __init__(self, deck):
        self.deck = deck
        self.index = 0

    def __iter__(self):
        return self

    def __next__(self):
        if self.index > len(self.deck):
            raise StopIteration

        value = self.deck.contents[self.index]
        self.index += 1
        return value

A more practical approach is to have Deck.__iter__ borrow some convenient iterator type. For instance, you could do return iter(self.contents) and you'd get an iterator that works exactly like the custom version above. Another option is to make __iter__ a generator function, since generator objects are iterators. This can be convenient if you need to do just a little bit of processing on each item as you iterate over it.

Collectives™ on Stack Overflow

Python Iterator class index overflows on second of for loop run

Code:

On running, I get the following error:

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

Code:

On running, I get the following error:

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related