Opened 13 years ago

Closed 13 years ago

Last modified 12 years ago

#13828 closed (fixed)

Refactor `dictsort` and `dictsortreversed` to make them more DRY

Reported by: Satoru Logic Owned by: Satoru Logic
Component: Template system Version: 1.2
Severity: Keywords: defaultfilters
Cc: satorulogic@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


In django.template.defaultfilters, dictsort and dictsortreversed are almost the same except for the invocation of reverse() on the sorted list in the later function.

Now I have defined another function _dictsort(value, arg, reverse=False), and make dictsort and dictsortreversed call it.

Attachments (3)

13828_dry_dictsort.diff (1.4 KB) - added by Satoru Logic 13 years ago.
13828_dry_dictsort_2.diff (1.5 KB) - added by Dougal Matthews 13 years ago.
13828_dry_dictsort_3.diff (3.4 KB) - added by Łukasz Rekucki 13 years ago.
Simplified patch.

Download all attachments as: .zip

Change History (9)

Changed 13 years ago by Satoru Logic

Attachment: 13828_dry_dictsort.diff added

comment:1 Changed 13 years ago by Paul McMillan

Triage Stage: UnreviewedAccepted

comment:2 Changed 13 years ago by anonymous

It appears the diff attached was created from the template directory. diff's should be created against the base django directory.

Changed 13 years ago by Dougal Matthews

Attachment: 13828_dry_dictsort_2.diff added

comment:3 Changed 13 years ago by Dougal Matthews

I've attached a patch that is the same but with the diff generated from the root directory.

I can also confirm that the patch cleanly applies to trunk and tests pass.

Changed 13 years ago by Łukasz Rekucki

Attachment: 13828_dry_dictsort_3.diff added

Simplified patch.

comment:4 Changed 13 years ago by Łukasz Rekucki

Summary: [patch] Refactor `dictsort` and `dictsortreversed` to make them more DRYRefactor `dictsort` and `dictsortreversed` to make them more DRY

Both functions turn into one-liners in Python 2.4, so a common function isn't needed. The previous patch also had a subtle change in behavior: the original list was modified by use of filter. Current patch returns a new list instead. Also modified tests to explicitly check that the original list isn't modified.

PS. There already is a "has patch" flag. You don't need to write "patch" in the topic.

comment:5 Changed 13 years ago by Alex Gaynor

Resolution: fixed
Status: newclosed

(In [15316]) Fixed #13828 -- DRY'd up the dictsort(reversed) filters, will speed them up as well.

comment:6 Changed 12 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

Note: See TracTickets for help on using tickets.
Back to Top