1

I am creating a class and trying to define class variables that correspond to a function like .keys() or .values() that are called on another class variable. For example:

class DATA(object):
    def __init__(self, id, database = {}):
        self.id = id
        self.database = database
        self.addresses = database.keys()
        self.data = database.values()

This does not seem to work, as when I create an instance of the class

foo = DATA(0,{"a":1,"b":2})

and then ask for:

print(foo.addresses)
>>> []

and it gives back an empty list.

Note:

On my actual program I start out with an empty dictionary for any class instance, then later on I use a function to add to the dictionary. In this case calling the ".database" still works but ".addresses" does not.

Can anyone help me with this problem?

Brandon H. Gomes
  • 808
  • 2
  • 9
  • 24
  • @Brandon Gomes: When you say 'and then ask for' -- is there actually some step inbetween? Did you actually start with `foo = DATA(0, {})` and then change DATA.database later? – Håken Lid Jan 16 '15 at 02:24

2 Answers2

1

I'm not sure that this is the problem, but using a mutable such as {} as a default argument often leads to bugs. See: "Least Astonishment" and the Mutable Default Argument

This is safer:

def __init__(self, id, database=None):
    if database is None:
        self.database = {}
    else: 
        self.database = database

I don't understand the purpose of DATA.addresses and DATA.data. Could you use functions with the property decorator instead, to avoid redundancy?

@property:
def addresses(self):
    return self.database.keys()

@property:
def data(self):
    return self.database.values()
Community
  • 1
  • 1
Håken Lid
  • 22,318
  • 9
  • 52
  • 67
  • Thanks Håken. I avoided using conditionals because I wanted the code to be more compact and possibly faster. If I use your suggestion then I get a "NoneType having no key attribute" error. I think if I just get rid of the mutable argument all together, it might work, since I won't ever use the database other than a dictionary. – Brandon H. Gomes Jan 16 '15 at 01:53
  • I don't think this explains it. I think there were some things you did in between defining `foo` and checking its addresses that deleted things. But it's still a much better idea to write the code this way. – Joel Jan 16 '15 at 01:54
  • And if you are worried that an if-block makes your code slow, then you need to be reminded of this quote from Donald Knuth. "Programmers waste enormous amounts of time thinking about, or worrying about, the speed of noncritical parts of their programs, and these attempts at efficiency actually have a strong negative impact when debugging and maintenance are considered. We should forget about small efficiencies, say about 97% of the time: premature optimization is the root of all evil. Yet we should not pass up our opportunities in that critical 3%." – Håken Lid Jan 16 '15 at 01:59
1

The issue is that you're calling keys right in your __init__ method, and saving the result. What you want to do instead is to call keys only when you want to access it.

Now, depending on the requirements of your class, you may be able to do this in a few different ways.

If you don't mind exposing changing the calling code quite a bit, you could make it very simple, just use foo.database.keys() rather than foo.addresses. The latter doesn't need to exist, since all the information it contains is already available via the methods of the databases attribute.

Another approach is to save the bound instance method database.keys to an instance variable of your DATA object (without calling it):

class DATA(object)
    def __init__(self, database=None):
        if database is None:
            database = {}
        self.database = database
        self.addresses = database.keys    # don't call keys here!

In the calling code, instead of foo.addresses you'd use foo.addresses() (a function call, rather than just an attribute lookup). This looks like a method call on the DATA instance, though it isn't really. It's calling the already bound method on the database dictionary. This might break if other code might replace the database dictionary completely (rather than just mutating it in place).

A final approach is to use a property to request the keys from the database dict when a user tries to access the addresses attribute of a DATA instance:

class DATA(object)
    def __init__(self, database=None):
        if database is None:
            database = {}
        self.database = database
        # don't save anything as "addresses" here

    @property
    def addresses(self):
        return self.database.keys()

This may be best, since it lets the calling code treat addresses just like an attribute. It will also work properly if you completely replace the database object in some other code (e.g. foo.database = {"foo":"bar"}). It may be a bit slower though, since there'll be an extra function call that the other approaches don't need.

Blckknght
  • 100,903
  • 11
  • 120
  • 169