On 16 Jan 2006, at 08:49, hugo wrote:
Uhm - in what way is nowadays defining classes and using them
"clumsy"?
Thought that's what OO is all about. The beast is named "removing the
magic" for a good reason, so why throw in another "magic" (one that
turns a scoped class definition into a - differently named! -
attribute)? And why should we do a full round again on the discussion
about wether we want managers the way we already decided in the
previous round to have them? Actually this discussion sounds
clumsy ;-)
We already have the bit of magic in there that says "if no Manager is
defined, set one up and call it objects". This would just alter that
rule to include the condition "and use the inner class called Manager
if it exists".
- multiple managers make it possible to have different ways to access
your data. Don't just think "multiple managers to the same storage" as
it is now, think "multiple managers to different - maybe even
technically different - storages". One thing I can see done with
multiple managers would be a way to define managers that link to
specific databases and so have a model definition where you have one
manager attaching to your PostgreSQL database and another attaching to
your RDF storage.
OK, I'm convinced that multiple managers can be useful. There's
nothing about the inner class proposal however that would stop you
from using them:
class Person(models.Model):
first_name = models.CharField(maxlength=30)
last_name = models.CharField(maxlength=30)
class Manager(models.Manager):
def get_list(self, **kwargs):
# Changes get_list() to hard-code a limit=10.
kwargs['limit'] = 10
return self.get_list(self, **kwargs)
class RDFManager(django.rdf.Manager):
def get_list(self, **kwargs):
kwargs['limit'] = 10
return super(RDFManager, self).get_list(self, **kwargs)
objects = Manager() # This isn't necessary, it will happen
automatically if omitted
rdf_objects = RDFManager()
- managers that are decoupled from the model class could be reused in
multiple models. For example I have a model with four quite similar
tables - even though they carry different payload, they do carry
similar basic structure for identification. They could definitely
share
the same manager definition that would just do basic selecting (like
limiting to objects from the current site).
That's possible with the inner class proposal as well:
class Person(models.Model):
first_name = models.CharField(maxlength=30)
last_name = models.CharField(maxlength=30)
Manager = MySpecialManagerClass
Or (based on the logic that objects is created only if it doesn't
already exist as a manager):
class Person(models.Model):
first_name = models.CharField(maxlength=30)
last_name = models.CharField(maxlength=30)
objects = MySpecialManagerClass()
That said, while there are workarounds for each of your objections,
put together they have convinced me that inner classes aren't nearly
as big a win as I thought they were. It also turns out you can use
them already if you want to just by explicitly instantiating them as
your objects property:
class Person(models.Model):
first_name = models.CharField(maxlength=30)
last_name = models.CharField(maxlength=30)
class Manager(models.Manager):
def get_list(self, **kwargs):
# Changes get_list() to hard-code a limit=10.
kwargs['limit'] = 10
return self.get_list(self, **kwargs)
objects = Manager()
That satisfies my desire for the stuff relating to the Person model
to live in the same class, and also means we don't have to add any
more magic to the framework.
Oh, and I strongly dislike the handling of an attribute named
'objects'
you propose. That's backwards into the discussion we already had. This
"you have to rename your attribute objects_ because we clobber your
namespace and won't allow you to reclaim it" felt like a hack to me
then and feels like a hack to me now. I am definitely -1 (it's more
like -2, if that would exist) on changing that to your proposal. The
current idea of explicitely instantiating and naming managers is much
cleaner.
Personally I find the idea of "every model class has an objects
property for manipulation in bulk, except when it doesn't" pretty
horrifying, especially from a code maintenance point of view. I guess
if people really want to make their lives more difficult we should
let them but I certainly wouldn't want it suggested as Django best
practise.
Overall then I think I'll withdraw the inner class proposal.
Cheers,
Simon
