Opened 6 years ago

Closed 6 years ago

Last modified 5 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: UI/UX:

Description

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 6 years ago.
13828_dry_dictsort_2.diff (1.5 KB) - added by Dougal Matthews 6 years ago.
13828_dry_dictsort_3.diff (3.4 KB) - added by Łukasz Rekucki 6 years ago.
Simplified patch.

Download all attachments as: .zip

Change History (9)

Changed 6 years ago by Satoru Logic

Attachment: 13828_dry_dictsort.diff added

comment:1 Changed 6 years ago by Paul McMillan

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

comment:2 Changed 6 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 6 years ago by Dougal Matthews

Attachment: 13828_dry_dictsort_2.diff added

comment:3 Changed 6 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 6 years ago by Łukasz Rekucki

Attachment: 13828_dry_dictsort_3.diff added

Simplified patch.

comment:4 Changed 6 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 6 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 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

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