Problems with a Python List

Question

Possible Duplicate:
“Least Astonishment” in Python: The Mutable Default Argument

Okay, so I am trying to learn Python. I had a friend who had an intro to Python class a few semesters ago and he gave me all of his old assignments. I am having a particularly annoying problem that I think should be quite simple, but can't seem to figure out where my problem lies. See what you folks think.

The program is supposed to read from a file called grades.txt. Here is the file:

2
John Doe
82
100
57
0
Jane Smith
91
12
45
81
0

The format of this file is: The first line is the number of students. Then there are the names of the students followed by their grades. The zero symbolizes the end of the student's list of grades. I know, I know...doesn't make a lot of sense to do it that way, but that is the way this reads.

Anyways, here is the code that I have written for it so far....

#!/usr/bin/env python

class students():
        def __init__(self, fname='', lname='', grades=[]):
        self.firstName = fname
        self.lastName = lname
        self.gradeBook = grades

def lineCheck(lineText):
     try:
         int(lineText)
         return True
     except ValueError:
         return False

inFile = "grades.txt"
outFile = "summary.txt"

count = 0
numStudents = 0

studentList = []
check = False

with open(inFile, "r") as file: 
    for line in file:
        if(count == 0):
            numStudents = line.strip()
        else:
            lineRead = line.strip()
            check = lineCheck(lineRead)
            if(check == False):
                studentName = lineRead.split()
                fName = studentName[0]
                lName = studentName[1]
                temp = students(fName, lName)
            else:
                if(lineRead != '0'):
                    temp.gradeBook.append(lineRead)
                elif(lineRead == '0'):
                    studentList.append(temp)

        count += 1
file.close()

for student in studentList:
    print student.firstName + " " + student.lastName
    print student.gradeBook

With this code, the expected output for me is at the end of the program in the final for loop. I expect to see something like this:

John Doe
['82', '100', '57']
Jane Smith
['91', '12', '45', '81']

However, the output I am getting is this:

John Doe
['82', '100', '57', '91', '12', '45', '81']
Jane Smith
['82', '100', '57', '91', '12', '45', '81']

I have been staring at this for way too long, and I have a feeling this is something very simple. But as I am new to python and have not gotten fully accustomed to all of its nuances yet, maybe someone with more experienced eyes can pick out what is going on here. I would really appreciate any help you can give me. Thanks.

Answer 1

Your actual problem is that you're using [] as a default argument. Default arguments are created once and "attached" to the function, so every time you create a new student object, you're reusing the same list of grades. You want something like this instead:

def __init__(..., grades=None):
    if grades is None:
        self.gradeBook = []
    else:
        self.gradeBook = grades

---

Now, allow me to give you some other critique. :)

It looks like your code has a combination of spaces and tabs in it; in a language like Python, where whitespace is important, this is really bad. I don't know what editor you're using, but you should find out how to configure it so pressing Tab always inserts four spaces.

The rest of this is rather less important, but it'll help you get along with other Python programmers, and might help your code come out simpler.

---

You do this in several places:

if(check == 0):

The parentheses are unnecessary.

if check == 0:

---

You have the following:

if(lineRead != '0'):
    ...
elif(lineRead == '0'):
    ...

But only one of these can be true—it's either equal or not equal—so you might as well replace the elif ... with else. But that leads to a double negative (it's not !=...), so let's swap the branches.

if lineRead == '0':
    ...
else:
    ...

---

check = lineCheck(lineRead)
if(check == False):

You only use the check variable once, so there's not much need for it. Also, it's more common to avoid comparing to True or False directly:

if not lineCheck(lineRead):

This doesn't read very well, though. Functions that perform an action are best named as verbs, and functions (like this one) that verify something sound decent when named is_whatever. Variables, you want to name like you're talking about it in English; you wouldn't often talk about a "line read", but you might talk about a "read line", or even just a "line".

Oh, and the Python style guide, PEP 8 advises using underscores instead of camelCase.

if not is_numeric(line):

---

Class names are conventionally written in UppercaseCamelCase, and should be singular, since they describe a kind of thing. ("What kind of pet do you have?" "A cat.") And you should always inherit from object, or you get an "old-style class", which hails from Python's early days and is a bit crufty.

And while I'm at it, it doesn't make much sense for a student to not have a name. Though you're better off not trying to split it into first and last.

class Student(object):
    def __init__(self, name, grades=None):
        ...

---

It doesn't matter right now, but it's helpful later on to put all your "main" code in a function called main and then run it. That way, you can import main from other code (like, say, a test) without having everything run right away.

if __name__ == '__main__':
    main()

__name__ is just the name of the current "module", and it's set to the special string '__main__' if you're running a file directly with python file.py.

---

After some heavy nitpicking, I ended up with this:

class Student(object):
    def __init__(self, name, grades=None):
        self.name = name
        if grades is None:
            self.gradebook = []
        else:
            self.gradebook = grades

def is_numeric(s):
    try:
        int(s)
        return True
    except ValueError:
        return False

def main(infile, outfile):
    count = 0
    num_students = 0

    students = []

    with open(infile, "r") as f:
        for line in f:
            line = line.strip()

            if count == 0:
                # This is the first line
                num_students = int(line)
            elif not is_numeric(line):
                # Non-numbers are names of new students
                cur_student = Student(line)
            elif line == '0':
                # Zero marks the end of a student
                students.append(cur_student)
            else:
                cur_student.gradebook.append(line)

            count += 1

    for student in students:
        print student.name
        print student.gradebook

if __name__ == '__main__':
    main("grades.txt", "summary.txt")

I did a few more little things not mentioned above; I hope they make sense.

• file is the name of a built-in function. Python will let you use the name, but you'll get a surprise if you try to use the built-in later on. I replaced it with f, which is a common way to refer to a file only open for a short while.

• It's hard to remember what temp means! Try not to give variables names like that. I changed it to cur_student, where "cur" is a common abbreviation for "current".

• You don't need f.close(); the with block does it for you. That's the whole point of with, in fact.

• You call line.strip() no matter what happens, so I put it as the first thing in the loop. That allows all the ifs to combine into one, which makes it easier to follow what's happening and when.

• I added a couple quick comments since, as you point out, this is a pretty weird file format.

This is a lot closer to how I would write it, if that means anything.

Always glad to see someone new get into programming! Hope you enjoy Python :)

Answer 2

Use

class students():
    def __init__(self, fname='', lname='', grades=None):
        self.firstName = fname
        self.lastName = lname
        self.gradeBook = [] if grades is None else grades

The problem is the default-argument []