For Loop iteration Problem

[VintageDude]

Why does this for loop only iterate 6 times. I just don't understand what is wrong.

Code: Select all

players = ['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R']
# define the add_player to team function
def add_player_to_team(team1, team2):
    # pick a player and add them to the team list and remove from available players
    #shuffle(players)
    if len(players) < 2:
        return "Not enough players."
    else:
        for z in players:
            # Pick Team 1
            player_picked = choice(players)
            team1.append(player_picked)
            players.remove(player_picked)
            # Pick Team 2
            player_picked = choice(players)
            team2.append(player_picked)
            players.remove(player_picked)

    return team1, team2

[pidd]

At a quick guess without testing (I'm useless at object orientated).

You are using z as an index for players, but you are reducing the size of players as you go along by deleting them.

[VintageDude]

Yes but each time through the loop reduces the list size by 2 items. There are 18 items total in the list. The for loop should loop 9 times. 18/2 = 9. the for loop only loops 6 times.

[jojopi]

It is never a good idea to change the length of a list inside a loop that is iterating over that list. In this case "for z in players" is probably counting how many items it has processed, but as you randomly remove players from the list, it is effectively jumping forwards.

The most general solution is to make a temporary list:

Code: Select all

remaining_players = players[:]
for z in players:
    …
    if bad(z):
        remaining_players.remove(z)
    …
players = remaining_players

The [:] is to ensure that we copy the items and make a new list, not just a new name for the existing list.

In your case I do not think you need a temporary copy. You never refer to the loop variable z inside the loop, and the player you choose is not even the one the for loop is thinking of. In fact, you are assigning two players per loop iteration. You really want "while len(players) >= 2".

Yes but each time through the loop reduces the list size by 2 items. There are 18 items total in the list. The for loop should loop 9 times. 18/2 = 9. the for loop only loops 6 times.

The for loop should iterate 18 times if there are 18 items in the list, or zero times if there are none. You are changing the number of items all the time.

The way to do it with a for loop that ignores its loop variable would be "for unused in range(len(players) // 2)".

[VintageDude]

for unused in range(len(players) // 2)

That works but I am going to have to look it up. to understand it. Thank you kind sir.

[pidd]

Yes but each time through the loop reduces the list size by 2 items. There are 18 items total in the list. The for loop should loop 9 times. 18/2 = 9. the for loop only loops 6 times.

index at 1 looks at #1
index at 2 looks at #4 (second item out of remaining 16, first two now missing)
index at 3 looks at #7 (3rd item out of remaining 14, first 4 now missing )
index at 4 looks at #10 (4th item out of remaining 12 items)
index at 5 looks at #13
index at 6 looks at #16 (6th item of remaining 8)
runs out because index is 7 and there are only 6 items left.

Not worth chasing moving targets, it gets confusing. If you want confusion it is a prime target for a recursive function.

[VintageDude]

Ahhhhh!!! Clarity! I see said the blind man as he picked up his hammer and saw.