Hi Jian, Apologies for the delay in responding.
On Thu, May 8, 2014 at 9:44 AM, Jian Li <[email protected]> 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 [email protected]. To post to this group, send email to [email protected]. 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.
