4
\$\begingroup\$

I'm attempting to apply multiple filters and maps to a list and attempted to refactor into something i believe is more readable. In my code, I am reading a file to create a list of terms, then munging it into something I will feed into another script.

Please note my terminology in the title might be wrong, but please feel free to correct.

Originally, I had something that looked like this:

random_substring = lambda word: word[:(random.randrange(len(word) - 2) + 1)]

only_longer_words = lambda word: len(word) > 2 

terms = []

#File manipulation that adds words to terms

terms = filter(only_longer_words, terms)
terms = map(random_substring, terms)
terms = filter(only_longer_words, terms)

#Save terms to file

After refactoring, I created a ChainList which inherits from list:

class ChainList(list):
    def filter(self, function):
        return ChainList(filter(function, self))

    def map(self, function):
        return ChainList(map(function, self))

    def uniquify(self):
        return ChainList(OrderedDict.fromkeys(self))

and use it as such:

terms = ChainList()

working_list = (
    terms.filter(only_longer_words)
         .map(random_substring)
         .filter(only_longer_words) 
         .uniquify()
)

The intent is to make it easy to chain operations on lists. It works, but as a Python beginner, wanted a review.

\$\endgroup\$
2
  • \$\begingroup\$ Obligatory link: legacy.python.org/dev/peps/pep-0008 \$\endgroup\$ Commented Sep 5, 2014 at 17:58
  • \$\begingroup\$ @jonrsharpe Thanks, I think I cleaned it up appropriately \$\endgroup\$ Commented Sep 5, 2014 at 18:18

2 Answers 2

Reset to default
2
\$\begingroup\$

The problem statement is "to apply multiple filters and maps to a list". This suggests that a good solution is one that takes the list as input applies the composition of operations to that list. Nothing in the problem statement suggests (to me) the creation of a new list-like object with new abilities. I would therefore take a different approach to this problem: stepwise construction of a composed function. (You may also want to look at Colin Winter's functional module.)

import collections
class Composer(object):
  def __init__(self):
    self.flst = []
  def __call__(self, lst):
    result = lst
    for func in self.flst:
      result = func(result)
    return result
  def add_map(self, func):
    self.flst.append( lambda x: map(func, x) )
  def add_filter(self, func):
    self.flst.append( lambda x: filter(func, x) )
  def uniquify(self):
    #assume hashable objects
    uniquifier = lambda x: collections.OrderedDict.fromkeys(x).keys()
    self.flst.append( lambda x: uniquifier( x ) )

#Example
c = Composer()
c.add_map(lambda x: x*x)
c.add_filter(lambda x: x<30)
c.uniquify()

x = range(10) * 2    
print(c(x))
\$\endgroup\$
3
  • \$\begingroup\$ is it just a matter of preference then? In my example, I have the capability to compose on the fly, whereas in your example you compose a discreet set of operations. \$\endgroup\$ Commented Sep 8, 2014 at 16:26
  • \$\begingroup\$ Here is one perspective on this: you introduce a new type of object whose current state is completely opaque because it is a side effect of previous method calls. In contrast, a Composer keeps a full list of the operations it will apply and application of a composer has no side effects. (You could make this more precisely true by using a Builder pattern to produce your composers.) \$\endgroup\$ Commented Sep 8, 2014 at 19:18
  • \$\begingroup\$ Perhaps there is a difference in style depending on use. In this example, I am using it as a script that i will be constantly tweaking to work with different data sets. If this were part of a more static code base, I can see your point. I appreciate your feedback which was exactly what I was looking for in posting this example. Thanks \$\endgroup\$ Commented Sep 10, 2014 at 14:59
3
\$\begingroup\$

The biggest problem with your code was the formatting issues (PEP8 violations), which you already fixed, with the exception of workingList which should be working_list.

Doc strings would be nice.

Instead of "uniquify", perhaps unique or even uniq sounds better.

In your first lambda you used word as the variable, in the second you used x. It would be more natural to unify.

This code doesn't make your ass look fat, I think.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks @janos, I updated the code sample with the changes to workingList and made the lambdas consistent. I was looking for comments related to creating a new list and chaining methods together. \$\endgroup\$ Commented Sep 5, 2014 at 19:18
  • \$\begingroup\$ Actually you're not supposed to edit the code in your post at all, as it invalidates the reviews. \$\endgroup\$ Commented Sep 5, 2014 at 19:21
  • \$\begingroup\$ Ah, got it. Sorry about that \$\endgroup\$ Commented Sep 5, 2014 at 19:26

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.