Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Reassign self in init

Tags:

python

I have a class that has lookup dict in the class body, which I store all the instances and with some key.

When I instantiate the instances I don't hold them in any variable or external dict, I store them in this lookup dict.

When somehow I instantiate an instance that is already in the dict, I reassign it to the one in the dict and update it, using it's new and old value in some other function.

I wonder if this is a good practice? Or I should refactor and make it have a external dict that hold the instances.

Is there any PEP guide to this kind of behavior?

Example:

class MyClass:
    all_instances = {}  # this dictionary is in local space and it's clear that
                        # the only interaction is with MyClass

    def __init__(self, unique_id, x, y, z):
        if unique_id not in MyClass.all_instances:
            self.unique_id = unique_id
            self.x = x
            self.y = y
            self.z = z
            MyClass.all_instances[unique_id] = self
        else:
            self = MyClass.all_instances[unique_id]
            self.update(x, y, z)

    def update(self, new_x, new_y, new_z):
        self.x = self.do_something_with(self.x, new_x)
        self.y = self.do_something_with(self.y, new_y)
        self.z = self.do_something_with(self.z, new_z)

    @staticmethod
    def do_something_with(old_value, new_value):
        # do something with old value and new and return some value
        value = (old_value + new_value) / 2  # more complicated than tht
        return value


while True:
    for id in get_ids():  # fetch ids from some database
        x, y, z = get_values(id)  # fetch values from some other database
        MyClass(id, x, y, z)

The databases where I get the ids and values is changing every time, so I can't know for sure if I'll get the ones I have already instantiate, or the values will be different.

The way I see, is that all the functionality of the class happens within itself, no need to have dictionaries laying around, making it unclear where they are interacting with any other part of the code.

This is how I would normally do, without reassigning it:

class MyClass:
    def __init__(self, x, y, z):
        self.x = x
        self.y = y
        self.z = z

    def update(self, new_x, new_y, new_z):
        self.x = self.do_something_with(self.x, new_x)
        self.y = self.do_something_with(self.y, new_y)
        self.z = self.do_something_with(self.z, new_z)

    @staticmethod
    def do_something_with(old_value, new_value):
        # do something with old value and new and return some value
        value = (old_value + new_value) / 2  # more complicated than tht
        return value

all_instances = {}  # this dictionary is now in global space
                    # and it's unclear if the only interaction is with MyClass
while True:
    for id in get_ids():  # fetch ids from some database
        x, y, z = get_values(id)  # fetch values from some other database
        if id not in all_instances:
            all_instances[id] = MyClass(x, y, z)
        else:
            all_instances[id].update(x, y, z)
like image 477
f.rodrigues Avatar asked Jul 14 '15 06:07

f.rodrigues


1 Answers

What you've got here is a singleton-like pattern. (I say singleton-like because you don't actually have just one instance, but a set of instances keyed off a couple of parameters). There are lots of reason why singletons (and global data in general) are a bad idea, so I'd vote for the external registry.

However, if after thinking about it a bit, if you decide that you do want to use an "internal" registry, you can do that using __new__.

class MyClass(object):

    all_instances = {}

    def __new__(cls, unique_id, x, y, z):
        instance = cls.all_instances.get(unique_id)
        if instance is not None:
            instance.is_initialized = True
        else:
            instance = super(MyClass, cls).__new__(cls, unique_id, x, y, z)
            instance.is_initialized = False
        return instance

    def __init__(self, unique_id, x, y, z):
        if self.is_initialized:
            return
        # Do initialization here.

Notice how __new__ is used to change how object creation happens. If we already have a registered object, we return it rather than creating a new object -- Otherwise, we call __new__ on the super class (object) which gives us a new (uninitialized) instance of MyClass. After __new__ has been called (and assuming that it returns an instance of MyClass), __init__ will be called. This can lead to __init__ being called multiple times for the same object (that's what all the is_initialized business is about.

The is_initialized bit is optional -- I'm assuming you don't want to reset x, y, z, etc. But if you do, you can leave it off.

like image 98
mgilson Avatar answered Sep 25 '22 12:09

mgilson