[unoti]

Comprehensions are great, but they can hurt readability and maintainability when taken too far. Most of the time you shouldn't be playing code golf with your code, iteratively seeking to pack more and more work into a single line of code. That makes the code harder to understand and harder to re-use.

While many of the examples in this helpful article are good, the first example with the octets is an excellent example of how not to do it. Look at the octet parsing code we ended up with in the article:

  # Snippet 1
  octets = [bbs[i:i+8] for i in range(0, len(bbs), 8)]
  

It's nice and tight. What does it do? I'd need to peer at it a moment and decode it, executing it in my head. This is subjective, but I think code should be self-explanatory; it's up to the computer to execute code, not people in their heads. Is there an off-by-one error in there? Here's another way to do it that'd be better.

  octets = chunks(bbs, 8)

That function chunks() is something that I keep in an iterutils package which I end up using all the time. It's intuitive, and it has a doctest that shows that we definitely don't have an off-by-one error. It's also easier to re-use than the first one. Maybe it also bears mentioning that chunks() works on an iterator, while the first solution needs to keep the whole thing in memory at once. Here's the chunks method I use:

    def chunks(collection, chunk_size):
        """Divides list l into chunks of up to n elements each.
            >>> l = range(75)
            >>> chunks(l,10)
            [[0, 1, 2, 3, 4, 5, 6, 7, 8, 9],
            [10, 11, 12, 13, 14, 15, 16, 17, 18, 19],
            [20, 21, 22, 23, 24, 25, 26, 27, 28, 29],
            [30, 31, 32, 33, 34, 35, 36, 37, 38, 39],
            [40, 41, 42, 43, 44, 45, 46, 47, 48, 49],
            [50, 51, 52, 53, 54, 55, 56, 57, 58, 59],
            [60, 61, 62, 63, 64, 65, 66, 67, 68, 69],
            [70, 71, 72, 73, 74]]
        """
        for i in xrange(0, len(collection), chunk_size):
            yield collection[i : i + chunk_size]

When you catch yourself playing code golf and trying to pack more and more meaning into a single line, look for ways that you can break the problem down into multiple components that use each other. This kind of functional decomposition is one of the things that makes functional programming so wonderful. Lots of times the intermediate steps in a complex expression have meaning and are useful on their own.

[kilburn]

I totally agree with you on the core issue.

However, I would reject your `chunks` function in a code review and tell you to use `grouper` form the itertools recipes [1].

More generally,any time I've ended up with a long list comprehension, the answer has been to "check itertools and see how you would describe this ugly comprehension in those terms"

[1] https://docs.python.org/3/library/itertools.html#itertools-r...

[unoti]

Grouper is a recipe, it's not part of itertools. It probably should be!

[thomasahle]

All of the recipes in the itertools documentation should pretty much be considered folklore imho. If you always use the 'zip((iterator,)n)' approach, people will recognize it and know immediately what you mean.

[lalaithion]

(The article was written about python3 so I'll nitpick about this)

There is no xrange function in python 3. Now range has the functionality of xrange.

[im3w1l]

That doesn't work on an iterator.

[unoti]

Cool! When you need it to work on an input that's an iterator, please submit a patch with a doctest. That's one of the important benefits of reusable code.

[mfgs]

In terms of pure readability I find breaking the list comprehensions helps in more complex cases (and is still valid Python), e.g.:

    octets = [bbs[i:i+8]
              for i in range(0, len(bbs), 8)
              if i%2 != 0]