current community

your communities

more stack exchange communities

Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite 
to_convert_files = [FileEnc(filename, getEncoding(filename)) for
                    filename in filenames if getEncoding(filename) != None]

The problem here is that getEncoding gets called twice for each accepted file name: once in the if clause and once in the expression preceding for. I know I can avoid this by splitting this into a generator expression that does the mapping and a list comprehension that does the filtering, but I was wondering if there is a way keeping it condensed in a one-liner.

share|improve this question

2 Answers 2

up vote 5 down vote accepted

Often, list comprehensions can aid readability. But here cramming all that behaviour into a single line is obfuscation. The Pythonic way is to do the simplest and most readable thing possible.

Here, this probably means writing a generator function that uses a normal for loop and skips over elements without an available encoding.

def encoded_files(files):
    for file in files:
        encoding = getEncoding(file)
        if encoding is not None:
            yield FileEnc(file, encoding)

to_convert_files = list(encoded_files(filenames))
share|improve this answer
    
+1 for reminding to not overuse list comprehensions –  ojdo Dec 21 '14 at 19:07
up vote 3 down vote

You could process a generator of two-tuples (filename, encoding); this will only call getEncoding once for each filename in filenames. Also, note that you should test for None by identity, not equality (see the style guide):

to_convert_files = [FileEnc(filename, encoding) for filename, encoding in 
                    ((f, getEncoding(f)) for f in filenames) if encoding is not None]
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.