# Counting which numbers fall in a certain bracket

I'm taking a text file that looks like this:

77, 66, 80, 81
40,  5, 35, -1
51, 58, 62, 34
0, -1, 21, 18
61, 69, 58, 49
81, 82, 90, 76
44, 51, 60, -1
64, 63, 60, 66
-1, 38, 41, 50
69, 80, 72, 75


and I'm counting which numbers fall in a certain bracket:

f = open("//path to file", "r")
split = text.split()

greater70=0
sixtys=0
fiftys=0
fortys=0
nomarks=0

for word in split:
temp = word.replace(',', '')
number = int(temp)
if number > 70:
greater70+=1
if number >= 60 and number <= 69:
sixtys+=1
if number >= 50 and number <= 59:
fiftys+=1
if number >= 40 and number <= 49:
fortys+=1
if number == -1:
nomarks+=1

print greater70
print sixtys
print fiftys
print fortys
print nomarks


Have I done it in an efficient way sensible way or do experienced Python users think it looks a little convoluted?

• Is 70 meant to be ignored or is that a mistake?
– crdx
Jun 7, 2013 at 9:32
• I'd be tempted to divide by 10 and use counter. Nov 24, 2016 at 6:18

Notes:

• Creating so many variables is a warning sign that they should be grouped somehow (an object, a dictionary, ...). I'd use a dictionary here.

• Consider learning some functional programming (as opposed to imperative programming) to apply it when it improves the code (IMO: most of the time). Here it means basically using collections.counter and generator expressions.

• Don't read whole files, read them line by line: for line in file_descriptor.

• There are programmatic ways of detecting intervals of numbers (simple loop, binary search). However, since you have here only 5 intervals it's probably fine just to write a conditional as you did. If you had, let's say 20 intervals, a conditional branch would be a very sad thing to see.

• Conditional branches are more clear when you use non-overlapping conditions with if/elif/else (effectively working as expressions instead of statements).

I'd write:

from collections import Counter

def get_interval(x):
if x > 70:
return "greater70"
elif 60 <= x < 70:
return "sixtys"
elif x >= 50:
return "fiftys"
elif x >= 40:
return "fortys"
elif x == -1:
return "nomarks"

with open("inputfile.txt") as file:
counter = Counter(get_interval(int(n)) for line in file for n in line.split(","))
# Counter({'sixtys': 10, 'greater70': 10, None: 7, ..., 'fortys': 4})

• Aren't those for clauses the wrong way around? I.e. shouldn't it be Counter(get_interval(int(n)) for n in line.split(",") for line in file)? Jun 23, 2013 at 23:42
• @user9876: no, I am pretty sure it's correct (the output is real, I run that code). A list-comprehension is written in the same order you'd write a normal for-loop. Jun 24, 2013 at 8:17

Looks good, but there are a few things I've noticed that could be better:

• You should use the with statement when working with files. This ensures that the file will be closed properly in any case.
• I would split the input string on ',' instead of replacing the comma with a space. This way, something like '-7,67,34,80' will be handled correctly. (It's a bit tricky because you want to split on newlines too, though. I did it using a list comprehension.)

int() will ignore any whitespace before or after the number, so int(' 50') works just fine.

• You can write those conditions as 60 <= number <= 69.
• Since the numbers can only fall into one of the five categories, it's clearer (and more efficient) to use elif instead of if.
• Assignments with = or += should have spaces around them in Python. (See PEP 8 for a Python style guide.)

This is how the changes would look:

with open("//path to file", "r") as f:
split = [word for line in f for word in line.split(',')]

greater70 = 0
sixties = 0
fifties = 0
forties = 0
nomarks = 0

for word in split:
number = int(word)
if number > 70:
greater70 += 1
elif 60 <= number <= 69:
sixties += 1
elif 50 <= number <= 59:
fifties += 1
elif 40 <= number <= 49:
forties += 1
elif number == -1:
nomarks += 1

print greater70
print sixties
print fifties
print forties
print nomarks


You might also want to think about how to avoid the repetition of code when you're calculating and printing the 40s, 50s and 60s (which, by the way, are spelt forties, fifties, sixties).

I also noticed that 70 falls into none of the categories, is that the way it's supposed to be?

1) Try changing your conditions like this: (no need to check two conditions)

if number > 69:
greater70+=1
elif number > 59:
sixtys+=1
elif number > 49:
fiftys+=1
elif number > 40:
fortys+=1
elif number == -1:
nomarks+=1


2) Always take care of closing file handler, or better to use with statements as other expert suggested.