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)
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.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With