nested for loop only iterates one time

Question

I am trying to create a counter that finds what it needs to count from one file and what it should count from another file. It opens file1 and finds a city and its population separated by a dash, and file2 shows a city name and the crime separated by a dash. When I hardcode the city name in it works fine, but when I try and do it using an if loop to find the city name, it will find how many times the first city comes up in the crime report, but no more after that. Help please

for line in file1:
    dash = line.find("-")
    variableCity = line[:dash]
    cityPop = line[dash + 1:]
    crimeCounter = 0
    for crime in file2:
        x = crime[:dash]
        if x == variableCity:
            crimeCounter += 1
    print("{} which has a population of {} has {} reported crimes".format(variableCity, cityPop, crimeCounter))

that is my code

file1:

Bothell-89232
Kent-97232
Tacoma-89333
Renton-98632
Redmond-64789
Seattle-76978

file2:

Kent-Theft
Tacoma-Break In
Seattle-Break In
Tacoma-Auto Break In
Federal Way-Auto Break In
Kent-Break In
Tacoma-Auto Break In
Federal Way-Auto Break In
Kent-Mugging
Kent-Break In
Federal Way-Break In
Renton-Break In
Renton-Auto Theft
Tacoma-Mugging
Seattle-Theft
Auburn-Auto Theft
Renton-Theft
Tacoma-Auto Theft
Kent-Mugging
Seattle-Auto Break In
Tacoma-Theft
Kent-Auto Theft
Seattle-Break In
Auburn-Mugging
Tacoma-Mugging
Auburn-Auto Theft
Auburn-Auto Theft
Seattle-Auto Theft
Federal Way-Mugging
Kent-Mugging
Renton-Auto Theft
Tacoma-Mugging
Auburn-Theft
Seattle-Auto Break In
Auburn-Mugging
Seattle-Theft
Auburn-Theft
Auburn-Auto Break In
Federal Way-Auto Break In
Seattle-Break In
Kent-Theft
Seattle-Auto Break In
Federal Way-Auto Break In
Kent-Auto Break In
Seattle-Auto Break In
Renton-Auto Break In
Kent-Auto Break In
Renton-Break In
Federal Way-Mugging
Seattle-Mugging
Renton-Mugging
Renton-Auto Break In
Tacoma-Mugging
Tacoma-Auto Theft
Seattle-Auto Break In
Kent-Auto Theft
Kent-Auto Theft
Federal Way-Mugging
Tacoma-Auto Theft
Federal Way-Theft
Tacoma-Auto Theft
Renton-Auto Theft
Seattle-Theft
Seattle-Auto Break In
Tacoma-Mugging
Tacoma-Auto Theft
Seattle-Break In
Federal Way-Theft
Seattle-Auto Break In
Auburn-Auto Break In
Auburn-Auto Break In
Tacoma-Break In
Seattle-Mugging
Renton-Theft
Auburn-Theft
Renton-Theft
Seattle-Auto Theft
Auburn-Mugging
Seattle-Break In
Kent-Mugging
Kent-Break In
Federal Way-Break In
Federal Way-Auto Theft
Auburn-Theft
Tacoma-Theft
Kent-Auto Break In
Auburn-Auto Theft
Seattle-Mugging
Kent-Theft
Kent-Mugging
Kent-Auto Break In
Seattle-Theft
Tacoma-Auto Theft
Renton-Theft
Renton-Break In
Auburn-Break In
Renton-Mugging
Renton-Mugging
Tacoma-Break In

Please note that in each file the next city appears on a new line

Does file1 and file2 one liners, or each city takes one line? — Vadim Key, Apr 28 '16 at 06:49
Sorry this is my first time using stacked overflow so I am having a hard time with the formatting! they are two nested for loops, and each city appears on a different line — sidfvdx125, Apr 28 '16 at 07:03
In the first `for` loop for `file1` you loop over the full content of `file2`. So the second time the outer loop runs `file2` is exhausted and the inner `for` loop won't run again. — Matthias, Apr 28 '16 at 08:12
@Matthias your comment is accurate and should be an answer. This issue is also resolved by my suggested solution, which only loops over `file2` once. — machine yearning, Apr 28 '16 at 11:10

Vadim Key · Answer 1 · 2016-04-28T07:06:09.210

Looks like you have missed finding the dash position in this piece:

for crime in file2:
    x = crime[:dash]

Shouldn't it be:

for crime in file2:
    dash = crime.find("-")
    x = crime[:dash]

Either way more correct solution should looks like:

for line in file1:
    parsed = line.split("-")
    variableCity = parsed[0]
    cityPop = parsed[1][:-1]

    file2 = open("file2.txt")
    crimeCounter = 0
    for crime in file2:
        c = crime.split("-")
        if c[0] == variableCity:
            crimeCounter += 1

    print("{} which has a population of {} has {} reported crimes".format(variableCity, cityPop, crimeCounter))

Yet more optimal solution should do it in two passes, in the first pass we are reading cities info to map and than increment crime reporting:

citiesPop = {}
citiesCrime = {}

for line in file1:
    parsed = line.split("-")
    city = parsed[0]
    cityPop = parsed[1][:-1]
    citiesPop[city] = cityPop
    citiesCrime[city] = 0

for crime in file2:
    city = crime.split("-")[0]
    if city in citiesCrime:
        citiesCrime[city] += 1

for city in citiesPop.keys():
    print("{} which has a population of {} has {} reported crimes".format(city, citiesPop[city], citiesCrime[city]))

I am using the dash found in the first for loop because I am looking for the city, so the length of the city name should be the same. Also my problem is that the second for loop only iterates one time — sidfvdx125, Apr 28 '16 at 07:05
@syanh7 please see edited solution, feel free to ask questions on it — Vadim Key, Apr 28 '16 at 07:08

co2y · Answer 2 · 2016-04-28T07:18:11.967

0

the code seems right, i guess something wrong with your file1 and file2. you can check the two variables or show the code about how to get file1 and file2

edited Apr 28 '16 at 07:18

answered Apr 28 '16 at 07:13

co2y

179
12

NanoTera · Answer 3 · 2016-04-28T09:20:07.027

0

If you are using file handles

ie somewhere earlier in the code you have lines like

file1=open('file1')
file2=open('file2')

you will need to seek back to the beginning of file2 before searching through it for the second and subsequent times

eg

add the line

file2.seek(0)

before the line

for crime in file2:

or after the line that prints the counters.

Otherwise the file pointer is left at the end if file and you wont get any "crimes" from it. Sort of like having to rewind a tape before you can play it again.

I am sure it would be more efficient to read the file contents into variables once and then this problem wouldnt occur but I guess if they are small files that might not make much difference. If they were huge files the memory usage might stop you from reading them in but I doubt they are that huge.

edited Apr 28 '16 at 09:20

answered Apr 28 '16 at 07:31

NanoTera

61
5

Feel like mine was an answer as he did use this method, oh well no big deal, Im just trying to get past 50 points so I can add comments ;-) @matthias hit on it as well. – NanoTera Apr 29 '16 at 00:46
I'm new to this myself but I think you @syanh7 are supposed to tick it or mark it up in some way, I haven't asked any questions yet myself so not sure. I'm not even sure if its appropriate for me to "ask" or comment like this. – NanoTera Apr 29 '16 at 07:03

score 0 · Answer 4 · answered Apr 28 '16 at 08:33

Let me offer some suggestions on how to clean up the code, so we might see through to where the bug is, and also some general python debugging tips.

For the first file, consider using variableCity, cityPop = line.split('-') in order to simply the logic of parsing. Simpler logic -> less bugs is my rule of thumb. Something like this:

for line in file1:
    variableCity, cityPop = line.split('-')

Or you could even put this in its own dictionary right away:

city_pops = dict(line.split('-') for line in file1)

Now you have no need to even nest your for loops! This has several advantages. Most importantly, now you have a data structure you can inspect in the interactive interpreter, to see if it looks right.

>>> city_pops
{'Tacoma': '89333', 'Redmond': '64789', 'Kent': '97232', 'Seattle': '76978', 'Renton': '98632', 'Bothell': '89232'}

If the data structure is too big, try just checking a few entries. You can also check how many entries with len(city_pops)

Great, divide and conquer! Now that you have the first file out of the way and you know it's being parsed correctly, we can move on to the second.

Let's use the dash-split technique again here. Also since you're doing counting, I would suggest the underused built-in collection Counter.

If you wanted to just count all the entries, you could do something like:

from collections import Counter
crime_rate = Counter(line.split('-')[0] for line in file2)

You can check what this looks like in the interpreter again, to make sure you're on the right track:

>>> crime_rate
Counter({'Seattle': 21, 'Tacoma': 18, 'Kent': 18, 'Auburn': 15, 'Renton': 15, 'Federal Way': 12})

Now you just need to filter out the cities you're not interested in. Make sure each city name is a key in your city_pops dictionary from earlier:

crime_rate = Counter(line.split('-')[0] for line in file2
                     if line.split('-')[0] in city_pops.keys())

The final result:

>>> crime_rate
Counter({'Seattle': 21, 'Tacoma': 18, 'Kent': 18, 'Renton': 15})

Moral of the story is, don't nest loops if you don't need to. It makes it harder to debug and it can increase the computational complexity of your program. Also make liberal use of the string.split() method and the Counter class. Lastly, comprehensions and generator expressions are almost always better than for loops.

Basically then your program boils down to 2 lines:

city_pops = dict(line.split('-') for line in file1)
crime_rate = Counter(line.split('-')[0] for line in file2
                     if line.split('-')[0] in city_pops.keys())

score 0 · Answer 5 · answered Apr 28 '16 at 21:48

I think I got it what I did was add file2.seek() So the new code is

for line in file1:
    dash = line.find("-")
    variableCity = line[:dash]
    cityPop = line[dash + 1:]
    crimeCounter = 0
    file2.seek(0)
    for crime in file2:
        x = crime[:dash]
        if x == variableCity:
            crimeCounter += 1
    print("{} which has a population of {} has {} reported  crimes".format(variableCity, cityPop, crimeCounter))

I did it this way because it only added one line of code to my original. Thank you for all of your answers.

nested for loop only iterates one time

5 Answers5