Python, I'm repeating myself a lot when it comes to for loops and there must be a better way

Question

Lets say I have three lists and I need to iterate through them and do some stuff to the contents.

The three lists are streaks_0, streaks_1, and streaks_2. For each list, I need to use different values specific to each list. For example, streak_0_num0s will not work in the streaks_1 for loop.

Is there a way to make these three for loops into one or at least a way to clean this up?

for number in streaks_0:
    if number == 0:
        streak_0_num0s += 1
    elif number != 0:
        streak_0_sum += number
streak_0_average = (streak_0_sum / (len(streaks_0) - streak_0_num0s))

for number in streaks_1:
    if number == 0:
        streak_1_num0s += 1
    elif number != 0:
        streak_1_sum += number
streak_1_average = (streak_1_sum / (len(streaks_1) - streak_1_num0s))

for number in streaks_2:
    if number == 0:
        streak_2_num0s += 1
    elif number != 0:
        streak_2_sum += number
streak_2_average = (streak_2_sum / (len(streaks_2) - streak_2_num0s))

of course: use a list of dictionaries for instance. But that would be best suited for http://codereview.stackexchange.com — Jean-François Fabre, May 31 '17 at 19:47
I'm voting to close this question as off-topic because http://codereview.stackexchange.com/ is the site to ask for code improvements — Jean-François Fabre, May 31 '17 at 19:47
Unrelated to your question: reduce "elif number != 0" to a simple "else". — jarmod, May 31 '17 at 19:48
You can use threading to make them all run at the same time if that appeals to you. — alex, May 31 '17 at 19:49
a list of lists may help with streaks[i] when is in range len(streaks) — kouty, May 31 '17 at 19:56
I was initially skeptical of @Jean-FrançoisFabre's close vote, but skimming the code, answers, and comments, the sheer number of improvements to be made leads me to agree that this question is much better suited for [CodeReview.SE]. The issues go far beyond the single question that's been focused on, and the code would benefit greatly from a full review in an environment where pointing out any issue you see is encouraged. (I'm assuming you're a beginner, Ari, so don't take that personally. But you do have a lot to learn here.) — jpmc26, Jun 01 '17 at 00:59

score 16 · Accepted Answer · answered May 31 '17 at 19:47

16

Why not use a function?

def get_average(streaks):
    streak_0_num0s = 0
    streak_0_sum = 0

    for number in streaks:
        if number == 0:
            streak_0_num0s += 1
        elif number != 0:
            streak_0_sum += number
    streak_0_average = (streak_0_sum / (len(streaks) - streak_0_num0s))
    print(streak_0_average)

get_average(streaks01)
get_average(streaks02)
get_average(streaks03)

answered May 31 '17 at 19:47

CodeLikeBeaker

20,682
14
79
108

1

A minor quibble, but surely your function as written should be named `print_average` instead of `get_average`, since it prints the result instead of returning it? Of course, having a function automatically print its result is usually a code smell anyway... – Ilmari Karonen Jun 01 '17 at 00:54
1

@IlmariKaronen Or better yet, have `get_average` actually return the value and put the `print` outside it. – jpmc26 Jun 01 '17 at 00:56
@IlmariKaronen very true, I basically took his code as written and made a quick example. But yes, it really should say print_average, or as jpmc26 pointed out, return the results and then do something with it – CodeLikeBeaker Jun 01 '17 at 12:25

BPL · Answer 2 · 2017-06-01T09:26:36.543

Your code can be easily simplified with a function like the below one:

def calculate_avg(lst):
    return sum(lst)/(len(lst)-lst.count(0))

or this one if you prefer:

def calculate_avg(lst):
    return sum(lst) / len([l for l in lst if l != 0])

and here's a little usage example:

streaks = [
    [1, 2, 3, 0, 0, 0, 0],
    [0, 0, 0, 4, 5, 6, 0],
    [0, 0, 6, 7, 8, 0, 0]
]

for index, streak in enumerate(streaks):
    print("avg(streak{})={}".format(str(index).zfill(2), calculate_avg(streak)))

score 7 · Answer 3 · answered May 31 '17 at 20:12

Write a function, that you can call multiple times:

def calculate_average(values):
    non_zeros = 0
    sum = 0

    for value in values:
        if value != 0:
            sum += value
            non_zeros += 1
    return sum / non_zeros

streak_0_average = calculate_average(streaks_0)
streak_1_average = calculate_average(streaks_1)
streak_2_average = calculate_average(streaks_2)

MSeifert · Answer 4 · 2017-05-31T23:26:31.863

As the others already said: When you find you repeat yourself a lot of times: try to create a function that can be re-used.

However it's always a good idea to have a look around and see if someone already implemented such a function. In your case you could use numpy.mean (numpy is a 3rd party module) or statistics.mean (statistics is a built-in module in python 3.4+).

The only thing they don't do by default is excluding the zeros, so you have to do that yourself:

import numpy as np

def average(streaks):
    streaks = np.asarray(streaks)
    streaks_without_zeros = streaks[streaks != 0]
    return np.mean(streaks_without_zeros)

streaks_0 = [1, 2, 3, 4, 0, 1, 2, 3]
print(average(streaks_0))  # 2.2857142857142856

or:

import statistics

def average(streaks):
    streaks_without_zeros = [streak for streak in streaks if streak != 0]
    return statistics.mean(streaks_without_zeros)

streaks_0 = [1, 2, 3, 4, 0, 1, 2, 3]
print(average(streaks_0))  # 2.2857142857142856

Python, I'm repeating myself a lot when it comes to for loops and there must be a better way

4 Answers4