Hi Jian, Apologies for the delay in responding.
On Thu, May 8, 2014 at 9:44 AM, Jian Li <j...@jianli.us> wrote: > Hi Russ, > > Thanks for the feedback! > > I believe the proposed code already addresses points 1-4. > > 1) The implementation does in fact return the correct natural key, > `(self.username,)`, for the User model. > > 2) When multiple fields have `unique=True`, the implementation does not > choose from among them; it returns a tuple containing *all* > non-auto-generated unique fields. > Ok - I can see how that could work. I don't necessarily agree that it's obvious, since it would produce an over specified key - but from a technical perspective, it would work. > 3) In the presence of multiple `unique_together` tuples, the > implementation arbitrarily chooses the first one. > > 4) In the presence of both `unique_together` fields and `unique` fields, > the implementation arbitrarily goes with the former. > My apologies - I didn't catch that in my scan of your implementation. > You can see the specific algorithm at line 1422 > <https://github.com/jianli/django/commit/b6d644b45c379cae83f7f2609525e616b62ade52#diff-507b415116b409afa4f723e41a759a9eR1422> > . > > The implementation always chooses a natural key which uniquely identifies > the relevant model. The choice may be arbitrary in the presence of multiple > cues, but so is any individually-implemented choice of a natural key. > > At the very worst, if the user does not agree with the choice for the > specific model, they can fall back to individually implementing the natural > key for that model; this is no worse than the current situation. So I would > argue that the base implementation proposed here is strictly better than > the status quo. > > In the course of serializing about a dozen different models, I've found > this to be a very useful base implementation that has saved me from the > tedium of writing many lines of boilerplate. > That's the thing - I don't see it as boilerplate. I see it as a behaviour that *should* be explicitly defined, not implicitly derived from a model definition. Here's another example why: Changes in schema. You build some models, and generate some fixtures. Then you add a new unique field to your model (or remove one). You try to load your fixtures - and the load fails, because the serialisation scheme has implicitly changed. Ok, sure - you could work your way out of the problem by introducing a temporary manual definition for the serialiser, but the whole problem would have been avoided if you'd just used a manual definition in the first place. This is one of those times where dogmatic adherence to DRY is a bad thing. DRY doesn't mean you don't ever type the same thing twice. It means that you don't ever express the same *concept* twice. And in this case, the structure of your natural keys is a concept that is independent of the uniqueness constraints in your model - they're related concepts, to be sure - but they're independent. So - I'm still -0. If I had to provide an alternative, I'd say that a Meta definition to allow you to explicitly define your natural key: class MyModel(Model): ... class Meta: natural_key = ('field1', 'field2') that then automatically rolls out into the appropriate definitions (but would be overridden by manually defined key functions) would be a preferable solution - but we're talking about 2 methods that are, for the most part, 2 lines of code each. I just don't see the overhead of defining natural key functions as something that is particularly onerous, and the overhead involved in building, maintaining and testing automated (or semi-automated) natural key functions just isn't worth it, IMHO. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at http://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAJxq848ATeV-YQV5MRK9R82pBRFubVQjtA%2B9FsJvAkMOVn45sA%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
