#24328: The new Options._get_fields() method needs to be cleaned up
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.8alpha1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: 1.8-beta | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by pirosb3):
Hi All,
First of all, thanks for raising these issues. I'll answer in the order
you listed your concerns:
- {{{ export_ordered_set }}} is necessary: as the calls to {{{
_get_fields() }}} are recursive, we want to keep track of the order each
field is inserted. This is usually important for form builders (admin,
ModelForms, ..). If throughout this process there is a model that
overrides a field, then we want to make sure the order is still
maintained. I perfectly agree that a list could be used in this case but I
preferred using an OrderedDict because it felt better in terms of
simplicity of code (at least to me, your ticket says otherwise) and
complexity (constant time insertion and update). Finally, these
OrderedDict instances are never returned by the public API, they are only
returned by the recursive calls (the ones without {{{
export_ordered_set=True }}})
- I agree the comment is a bit misleading. The previous {{{ get_fields()
}}} was caching lists instead of tuples and was prone to cache
manipulation and I added this comment just for reference.
- Same answer as N.1
- Very correct! This comments refers to the previous implementation. I
will open a PR.
I hope my explanations were clear, but in case you would like to discuss
some other changes to put in the PR please let me know.
Dan
--
Ticket URL: <https://code.djangoproject.com/ticket/24328#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" 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].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.ddee6e9b75049b29bb519e65aed78c8b%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
