Opened 14 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

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 14 years ago.
13828_dry_dictsort_2.diff (1.5 KB ) - added by Dougal Matthews 14 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)

by Satoru Logic, 14 years ago

Attachment: 13828_dry_dictsort.diff added

comment:1 by Paul McMillan, 14 years ago

Triage Stage: UnreviewedAccepted

comment:2 by anonymous, 14 years ago

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

by Dougal Matthews, 14 years ago

Attachment: 13828_dry_dictsort_2.diff added

comment:3 by Dougal Matthews, 14 years ago

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.

by Łukasz Rekucki, 13 years ago

Attachment: 13828_dry_dictsort_3.diff added

Simplified patch.

comment:4 by Łukasz Rekucki, 13 years ago

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 by Alex Gaynor, 13 years ago

Resolution: fixed
Status: newclosed

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

comment:6 by Jacob, 12 years ago

milestone: 1.3

Milestone 1.3 deleted

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