views:

75

answers:

6

I've got this code:

def __parse(self):        
    for line in self.lines:
        r = Record(line)
        self.records[len(self.records):] = [r]
        print self.records[len(self.records)-1].getValue() # Works fine!
    print self.record[0].getValue() # Gives the same as
    print self.record[1].getValue() # as
    # ... and so on ...
    print self.record[len(self.record)-1].getValue()

Now what it should do is making records out of lines of text. But when I access those list after the for-loop has completed all records give the same results for methods I call on them. When I access a record within the for-loop right after it was appended it's the right one so the Record init can't be fault. No, it's absolutely sure that the lines I put in are different! Has anyone an idea why this happens? Help would be very appreciated!

+1  A: 

You aren't appending to self.records; you are always overwriting it.

Use:

self.records.append(r)

instead.

Edit: Never mind. See Ignacio Vasquez-Abrams's comment. I would delete this answer if not for that.

danben
Actually, `L[len(L):] = [X]` is a pathological form of append.
Ignacio Vazquez-Abrams
That is some pretty decent obfuscation.
Will McCutchen
Ah, I see it now. Wow.
danben
+1  A: 

Does it still happen if you replace it with the following:

self.record = [Record(l) for l in self.lines]

EDIT:

Something must be wrong in Record since the code there does work, even if it makes experienced coders weep when they read it.

Ignacio Vazquez-Abrams
I also think it is the Record() class which is broken. I really wonder how it is implemented… Seeing the `self.records[len(self.records):] = [r]` (valid!) code I guess it may be something 'clever' too :)Similar problems (overwriting some 'other' data) happen in case of mutable vs. immutable structure confusion and e.g. when using lists as default arguments.
Jacek Konieczny
A: 

The Record class is broken, you are always returning the same object.

Without seeing the code for Record it's impossible to guess

Perhaps you are using a list or a dict as default parameter to __init__ and returning that with getValue().

Another possibility is that getValue() is returning a class attribute rather than an instance attribute

gnibbler
A: 

Ok, so I'll post the code for the Record class for clarification, too.

class Record:

record = {}
line = ""

def __init__(self,line):
    self.line = line 
    self.__parse()

def __parse(self):
    fieldnames = ['from','to','value','error']
    fields = self.line.split(',')

    c = 0
    for field in fields:
        self.record[fieldnames[c]] = field.strip()
        c+=1

    self.record['from'] = datetime.datetime.strptime(self.record['from'],"%Y-%m-%d")
    self.record['to'] = datetime.datetime.strptime(self.record['to'],"%Y-%m-%d")

class CsvSet:

records = []

def __init__(self,lines):
    self.__parse()

def __parse(self):        
    for line in self.lines:
        self.records.append(Record(line))

The __parse method in CsvSet is now how it was in the beginning. I changed if for debugging reasons but the result is the same. And Ignacio you're right, I startet with Python only 2 weeks ago...

Ahue
You should edit your original question instead of adding this as an answer.
Jacek Konieczny
Thanks, I'll do it next time. Sorry!
Ahue
+1  A: 

Ahue, you have mutable objects in the shared class namespace -- a very common misconception when starting out with python. Move the initialization of records = [] in CsvSet into its __init__ function, and move record = {} into Record __init__ function. Should look like the following:

class Record:
    def __init__(self,lines):
        self.record = {}
        self.__parse()

class CsvSet:
    def __init__(self,lines):
        self.records = []
        self.__parse()

When you declare a mutable variable in the class area, it is shared among all instances of those classses, not created for each instance. By moving the initialization into an instance method (__init__ in this case), you are creating new mutable stores for each instance, which is what you intended.

Shane Holloway
Thank you very much. I thought it was like in Java. Now it seems to work!
Ahue
+1  A: 

Record class is broken. You use a class variable (Record.record) instead of an instance attribute. Class variable is one for all instances and you want different self.record for each instance.

Move the:

record = {}
line = ""

lines into the constructor (indented under def __init__(self,line):)

Jacek Konieczny