Set part of a lambda function in advance to avoid repeated code

Question

The following sorting method works perfectly.

def sort_view_items(self):

    cs = self.settings.case_sensitive

    if self.settings.sort_by_file_name:
        sk = lambda vi: (vi.name if cs else vi.name.lower(), vi.group, vi.tab)

    elif self.settings.sort_by_folder:
        sk = lambda vi: (vi.folder, vi.name if cs else vi.name.lower())

    elif self.settings.sort_by_syntax:
        sk = lambda vi: (vi.syntax, vi.name if cs else vi.name.lower())

    elif self.settings.sort_by_indexes:
        sk = lambda vi: (vi.group, vi.tab)

    self.view_items.sort(key = sk)

However the case sensitive related section of the lambdas vi.name if cs else vi.name.lower() gets used 3 times which irks my repeated code gene.

Out of interest, can the case aspect be set in advance somehow, but without making permenant changes to the name attribute or doing so in a temporary copy of the view_items list?

For example I tried using a lambda within a lambda which I didn't think would work and, guess what, it didn't. Although unexpectedly the syntax was accepted (no exceptions), it just didn't result in any sorting being actually performed.

def sort_view_items(self):

    cs = self.settings.case_sensitive

    name_lambda = lambda vi: vi.name if cs else vi.name.lower()

    if self.settings.sort_by_file_name:
        sk = lambda vi: (name_lambda, vi.group, vi.tab)

    ...

Note that defining a lambda and assigning it to a name like that defeats the whole purpose of lambda functions, and is specifically cautioned against in PEP-8. — TigerhawkT3
– TigerhawkT3, Commented Aug 7, 2016 at 14:20
It's a little more efficient to define your key functions outside sort_view_items. When you define functions inside another function those inner functions get re-compiled every time you call the outer function. — PM 2Ring
– PM 2Ring, Commented Aug 7, 2016 at 15:02
Ah, rightio. In that case, what you're doing is fine. :) Although as Tigerhawk said, lambdas are supposed to be anonymous, and you should use a proper def function if you don't want an anonymous function, although I'll concede that this is a borderline case. — PM 2Ring
– PM 2Ring, Commented Aug 7, 2016 at 15:17
@mattst. Both. According to the PEP-8 style guide, if you create an anonymous function and the give it a name, you're doing it wrong. See python.org/dev/peps/pep-0008/#programming-recommendations — PM 2Ring
– PM 2Ring, Commented Aug 7, 2016 at 15:45
@mattst You can use a def statement where you are currently assigning the result of a lambda expression to a name (def sk(vi): return ... instead of sk = lambda vi: ...). — chepner
– chepner, Commented Aug 7, 2016 at 20:51

flornquake · Accepted Answer · 2016-08-07 14:11:00Z

5

You need to actually call name_lambda:

sk = lambda vi: (name_lambda(vi), vi.group, vi.tab)

In your snippet, name_lambda is defined correctly but it is never going to be called.

answered Aug 7, 2016 at 14:11

flornquake

3,4761 gold badge24 silver badges34 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

chepner · Accepted Answer · 2016-08-07 16:38:11Z

5

This requires you to add a new property, "lower_name", to your class, but this one change lets you greatly simplify the rest of the code.

from operator import attrgetter

class Things(object):
    @property
    def lower_name(self):
        return self.name.lower()

    def sort_view_items(self):
        name_field = "name" if self.settings.case_sensitive else "lower_name"

        if self.settings.sort_by_file_name:
            fields = (name_field, "group", "tab")
        elif self.settings.sort_by_folder:
            fields = ("folder", name_field)
        elif self.settings.sort_by_syntax:
            fields = ("syntax", name_field)
        elif self.settings.sort_by_indexes:
            fields = ("group", "tab")

        self.view_items.sort(key=attrgetter(*fields))

edited Aug 7, 2016 at 16:38

answered Aug 7, 2016 at 15:57

chepner

538k77 gold badges594 silver badges746 bronze badges

2 Comments

Veky Over a year ago

You should name your name (in the first line of sort_view_items) name_field to be consistent with the rest of the code. Otherwise, great usage of attrgetter. And please use .casefold for casefolding instead of .lower (if your Python is new enough).

chepner Over a year ago

Thanks for the bug fix, and good suggestion for using casefold if available.

Kasravnd · Accepted Answer · 2016-08-07 21:54:56Z

1

Since you want to use it in 3 of 4 conditions, the best way for refusing of this repetition, is computing the name at the top of your if conditions. Also You can use a def key word to create your key function properly, and return the corresponding value, istead of defining a function each time. In this case you can pass the vi to key_func and calculate the name the the top level of this function.

def sort_view_items(self):

    def key_func(vi):
        name = vi.name if self.settings.case_sensitive else vi.name.lower()
        if self.settings.sort_by_file_name:
            return name(vi), vi.group, vi.tab

        elif self.settings.sort_by_folder:
            return vi.folder,name(vi)

        elif self.settings.sort_by_syntax:
            return vi.syntax, name(vi)

        elif self.settings.sort_by_indexes:
            return vi.group, vi.tab

    self.view_items.sort(key=key_func)

edited Aug 7, 2016 at 21:54

answered Aug 7, 2016 at 14:36

Kasravnd

108k19 gold badges167 silver badges195 bronze badges

5 Comments

mattst Over a year ago

That throws an exception: NameError: global name 'vi' is not defined

Kasravnd Over a year ago

@mattst Yeah, that's because you don't have vi in your function name space, let me update!

PM 2Ring Over a year ago

Also that's less efficient, since it has to do all those if / elif tests for every key.

Kasravnd Over a year ago

@PM2Ring As I said, since we need the name in 3 of 4, it's reasonable to do this but still it's not a proper way to go, check out my last update for the more pythonic way.

PM 2Ring Over a year ago

Your latest update still performs the if / elif tests for every element in the list being sorted. Of course, it may not perform all 4 tests, as it stops as soon as a test condition is True. But still...

Collectives™ on Stack Overflow

Set part of a lambda function in advance to avoid repeated code

3 Answers 3

Comments

2 Comments

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

2 Comments

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related