OO Bug needs squashing

[nick]

Ok I'm more than willing to admit it's probably me but I am struggling
with what appears to be a weird scope issue.

If you look at the code below I believe day 2 should only contain a
single event. But when you run the prog it prints the events from day
1 as well.

Any advice greatly appreciated.

class Day:
name = None
events = []
eventD = {}

class Event:
start = ''
end = ''
desc = ''

def loadMonth():
#Create Day one
day1 = Day()
day1.name = 'day1'

#Create first event for day one
event1 = Event()
event1.start = '10:00'
event1.end = '12:00'
event1.desc = 'event1'
day1.events.append(event1)

#Create second event for day one
event2 = Event()
event2.start = '14:00'
event2.end = '16:00'
event2.desc = 'event2'
day1.events.append(event2)

#Create a second day
day2 = Day()
day2.name = 'day2'

#Create a single event for day two
event3 = Event()
event3.start = '10:00'
event3.end = '12:00'
event3.desc = 'event3'
day2.events.append(event3)

#print day names to show they are different days
print day1.name
print day2.name

#print events for day 2 ONLY and it shows all day one as well ??????
for event in day2.events:
print event.desc


if __name__ == '__main__':
loadMonth()

 

[David Goodger]

> Ok I'm more than willing to admit it's probably me but I am
> struggling with what appears to be a weird scope issue.
>
> If you look at the code below I believe day 2 should only contain a
> single event. But when you run the prog it prints the events from
> day 1 as well.
>
> Any advice greatly appreciated.

First, don't use hard tabs. The standard is to use 4 spaces per
indent level.

> class Day:
> name = None
> events = []
> eventD = {}

These are class attributes. They're shared by all instances of
that class. "events" and "eventD" are mutable, which means that
they can change.

Instead, move events and eventD into an initializer method:

def __init__(self):
self.events = []
self.eventD = {}

You should also use the __init__ method to populate the object:

def __init__(self, name):
self.events = []
self.eventD = {}
self.name = name

> class Event:
> start = ''
> end = ''
> desc = ''

def __init__(self, start, end, desc):
self.start = start
self.end = end
self.desc = desc

> def loadMonth():
> #Create Day one
> day1 = Day()
> day1.name = 'day1'

Instead of the last 2 lines, use:

day1 = Day('day1')

> #Create first event for day one
> event1 = Event()
> event1.start = '10:00'
> event1.end = '12:00'
> event1.desc = 'event1'

Instead of the last 4 lines, use:

event1 = Event('10:00', '12:00', 'event1')

> day1.events.append(event1)

That should be a method instead:

day1.add_event(event1)

The method would look like this (add under "class Day"):

def add_event(self, event):
self.events.append(event)

That way, if you choose to change the implementation of
the events storage later, it will be easy.

And so on. I suggest you look at these pages:

* Python Tutorial: http://www.python.org/doc/current/tut/
* the FAQ: http://www.python.org/doc/faq/ (especially question 4.21 of
http://www.python.org/doc/faq/general.html, which directly answers
your original question)
* lots more links here: http://www.python.org/topics/learn/

-- David Goodger

 

[Paul Rubin]

class Day:
name = None
events = []
eventD = {}

I think you mean:

class Day:
def __init__(self):
self.name = None
self.events = []
self.eventD = {}

and similarly for the Event class.

That creates those attributes in each newly-created instance. The way
you have it, it makes attributes on the entire class, not separately
for each instance.

 

[nick]

I should know better but just couldn't see it.

Thanks, consider it squashed.