On Jan 25, 2006, at 9:57 PM, Adrian Holovaty wrote:
I've written up my latest proposal here:
http://code.djangoproject.com/wiki/DescriptorFields
Yum!
I agree with Joseph -- I like this new syntax a *lot*.
I do have a few semantic questions, though. WARNING: Much nit-
picking follows; I should lead with the disclaimer that I'm *very*
happy with the syntax; I just want to make sure we get it as perfect
as possible!
First:
Also, it would be cool if the _set stuff wasn't forced on us, there's
a disconnect between creating an attribute called 'sites' in your
model, and accessing that attribute via 'site_set'.
I *very* much agree with this.
Consider::
class Article(models.Model):
headline = models.CharField(maxlength=50)
reporter = models.ForeignKey(Reporter)
sites = models.ManyToManyField(Site)
The fact that I can't do ``article.sites.all()`` seems wrong -- is
there a reason for the foo_set name over the given field name that
I'm missing? This also deals with the edge case where I have
something like::
class Foo(models.Model):
bars = models.ManyToManyField(Bar)
bar_set = models.CharField()
Second, I'm still unhappy that it's not clear when a database lookup
is performed. Luckily, I think the proposed syntax makes it easy to
write a few rules so that you'll always know when the db is being
hit. How does this sound:
1. ``Manager.get`` always perfoms a lookup.
2. Any other manager method only actually hits the database when
iterated.
3. ``object.foreign_key`` always performs a lookup.
4. ``object.m2m_set`` is esentially a manager, so it behaves as
for 1 and 2.
There is a question of how/weather to cache database lookups::
article = Article.objects.get(pk=1)
article.reporter.fname # obvious performs a lookup
article.reporter.lname # does this?
I'd expect that subsequent accesses to the same foreign key do not
perform additional lookups, but if that's so there needs to be a way
to clear the cached object in case you have an object you need to
hang on to for a while. I'd suggest ``article.clear_relation_cache()``.
There's also the question of caching many to many lookups; my gut is
that Django shouldn't::
list(article.site_set) # hits the db
list(article.site_set) # does it again
Finally, it seems like the manager has a lot of methods, and I think
some of them are redundant. For one, ``Reporter.objects.all
().distinct()`` strikes me as less obvious than ``Reporter.objects.all
(distinct=True)``
In fact, I have trouble understanding why we need ``all()`` at all,
actually; what's the difference between these two?
::
Reporter.objects.all()
Reporter.objects.filter()
Why not rename "filter" to "list" (or "iterator" if we want to be
pedantic) and collapse the two methods into one?
Similarly, the chaining of filter/order_by/in_bulk seems crufty to
me; are these equivalent?
::
Reporter.objects.filter(fname="Joe").order_by("fname")
Reporter.objects.order_by("fname").filter(fname="Joe")
I'm a big fan of the "One True Way" principle, so that rubs me the
wrong way. Why not simply have order_by/distinct/etc. be kwargs to
the manager functions? If there's a good reason to do the "chained
methods" that's fine, but let's not be too clever for our own sakes, eh?
Adrian, thanks again for thinking this through -- I think this change
will make code a *lot* more readable, and of course a lot more fun.
Jacob
