0
\$\begingroup\$

I am currently going through Codecademy's Python course (I don't come from a coding background but I'm thoroughly enjoying it) and I got to the end of one of the sections and thought, "Well it worked, but that seemed wildly inefficient." I tried shortening unnecessary parts and came up with this:

lloyd = {
    "name": "Lloyd",
    "homework": [90.0, 97.0, 75.0, 92.0],
    "quizzes": [88.0, 40.0, 94.0],
    "tests": [75.0, 90.0]
}
alice = {
    "name": "Alice",
    "homework": [100.0, 92.0, 98.0, 100.0],
    "quizzes": [82.0, 83.0, 91.0],
    "tests": [89.0, 97.0]
}
tyler = {
    "name": "Tyler",
    "homework": [0.0, 87.0, 75.0, 22.0],
    "quizzes": [0.0, 75.0, 78.0],
    "tests": [100.0, 100.0]
}

classlist = [lloyd, alice, tyler]

# Add your function below!

def get_average(student):
    average_homework = sum(student["homework"]) / len(student["homework"])
    average_quizzes = sum(student["quizzes"]) / len(student["quizzes"])
    average_tests = sum(student["tests"]) / len(student["tests"])
    average_total = average_homework * 0.1 + average_quizzes * 0.3 + average_tests * 0.6
    return average_total

def get_letter_grade(score):
    if score >= 90:
        return "A"
    if score < 90 and score >= 80:
        return "B"
    if score < 80 and score >= 70:
        return "C"
    if score < 70 and score >= 60:
        return "D"
    else:
        return "F"

def get_class_average(classlist):
    a = []
    for student in classlist:
        a.append(get_average(student))
    return sum(a) / len(a)

get_class_average(classlist)

Now this bit works, but I was wondering if there are more places I could trim the code down without losing the functionality (I'm guessing there is in the get_average function but the things I tried came back as errors because of the "name" field in the dictionary). Codecademy seems good for learning the fundamentals, but I wanted to get some insight into the "best" way to do things. Should I not worry about reworking functional code to be more efficient at first and just learn the fundamentals, or should I keep trying to make the "best" code as I learn?

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Can you add information about the problem you are trying to solve ? \$\endgroup\$
    – SylvainD
    Commented Jul 7, 2013 at 14:50
  • \$\begingroup\$ @Josay Actually he isn't trying to solve any problem. This is the part of the python track on codecademy.com. One introducing to dictionary in Python. \$\endgroup\$ Commented Jul 7, 2013 at 15:06

3 Answers 3

1
\$\begingroup\$

In average_student, you're manually iterating over the keys in a dictionary, and you've hardcoded the weights of the various parts. What if you want to add a "presentation" component to the grade? You've got a lot of places to touch.

Consider exporting the hardcoded information into its own dictionary, and iterating over the keys in that dictionary:

weights = {
    "homework": 0.1,
    "quizzes": 0.3,
    "tests": 0.6
}

def get_average(student):
    return sum(average(student[section]) * weights[section] for section in weights)

def average(x):
    return sum(x)/len(x)

In terms of learning Python, I'd definitely agree with your instinct -- you should keep trying to make your code as clean as possible even while completing exercises. When you're learning fundamentals, it's that much more important to understand good structure.

\$\endgroup\$
3
  • \$\begingroup\$ Is this the get_average function? I tried but couldn't run it. \$\endgroup\$ Commented Jul 7, 2013 at 16:10
  • \$\begingroup\$ Oops! I had a syntax brainfart. Fixed it. \$\endgroup\$
    – llb
    Commented Jul 7, 2013 at 17:40
  • \$\begingroup\$ @llb Thanks for this input too! get_average was the section I didn't like when I got done with it either. Exactly like you said that whole section was basically hardcoded in there. I'll definitely be playing with this today as well! \$\endgroup\$
    – Raynman37
    Commented Jul 7, 2013 at 17:45
1
\$\begingroup\$
  • In the function get_letter_grade you were checking more conditions then necessary. Try to follow the logical difference between your function and my code. It is simple math which I used to optimize it. Also structuring it as an if-elif-...-else is better for readability.
  • In get_average and get_class_average I use a thing called list comprehension. A simple google search will tell you what it is and why it is better. Google it in python tutorials if you have problem.

Edited Functions

def average(values):
    return sum(values) / len(values)

def get_average(student):
    keys = ['homework', 'quizzes', 'tests']
    factors = [0.1, 0.3, 0.6]
    return sum([ average(student[key]) * factor for key, factor in zip(keys, factors)])

def get_letter_grade(score):
    if score >= 90:
        return "A"
    elif score >= 80:
        return "B"
    elif score >= 70:
        return "C"
    elif score >= 60:
        return "D"
    else:
        return "F"

def get_class_average(classlist):
    return average([get_average(student) for student in classlist])

get_class_average(classlist)
\$\endgroup\$
10
  • \$\begingroup\$ I would vote you up, but I don't have 15 reputation. This is exactly what I was meaning. It works to Codeacademy standards, but could easily be refined. Thanks for the list comprehension pointer as well! \$\endgroup\$
    – Raynman37
    Commented Jul 7, 2013 at 15:21
  • \$\begingroup\$ @Raynman37 Edited the answer a bit more. Try to figure out how the return statement of get_average works. Yeah you cannot vote up but you can accept an answer. \$\endgroup\$ Commented Jul 7, 2013 at 15:34
  • 3
    \$\begingroup\$ Actually you don't need a list-comprehension. A generator expression works too, and avoids the creation of the intermediate list. \$\endgroup\$
    – Bakuriu
    Commented Jul 7, 2013 at 15:34
  • \$\begingroup\$ @Bakuriu Yeah that would have been better. There are two reasons I didn't do that. FIrstly, the OP is a beginner and I don't think introducing generators to a beginner would be a good idea. Secondly, I am not much experienced in Python either so my explanation might not have been the best. If he is interested here is a link to a question on stackoverflow \$\endgroup\$ Commented Jul 7, 2013 at 15:39
  • 1
    \$\begingroup\$ I would have used zip in get_average(): return sum([average(student[key]) * factor for key, factor in zip(keys, factors)]). \$\endgroup\$
    – fjarri
    Commented Jul 8, 2013 at 3:19
0
\$\begingroup\$

Defining an average function (based on this), you could have things slightly clearer :

def average_list(l)
    return sum(l) / float(len(l))

def average_student(student):
    average_homework = average_list(student["homework"])
    average_quizzes =  average_list(student["quizzes"])
    average_tests =    average_list(student["tests"])
    average_total = average_homework * 0.1 + average_quizzes * 0.3 + average_tests * 0.6
    return average_total

def class_average(students):
    return average_list([average_student(s) for s in students])
\$\endgroup\$
4
  • \$\begingroup\$ You are not using the description in the link that you provided. You just added a function call which can be used in the other function as well. \$\endgroup\$ Commented Jul 7, 2013 at 15:07
  • \$\begingroup\$ Actually I was talking about using reduce and lambda. Anyways you don't need to use float in the average_list function either. All the values are float already so sum would also be. Implicit conversion so explicit isn't needed. \$\endgroup\$ Commented Jul 7, 2013 at 15:52
  • 1
    \$\begingroup\$ @Josay Are we supposed to use class in the class_average section because it's already a built in Python term (I don't know what you'd call it officially)? \$\endgroup\$
    – Raynman37
    Commented Jul 7, 2013 at 17:49
  • \$\begingroup\$ Raynman37 : As per docs.python.org/release/2.5.2/ref/keywords.html this is a valid comment. It completely went out of my mind but I'm going to change this. \$\endgroup\$
    – SylvainD
    Commented Jul 7, 2013 at 18:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.