Sourcery refactoring readability: "Use the built-in function next instead of a for-loop"

I am using Sourcery tool to suggest me some code simplifications and readability improvements. Today I got a suggestion to use next() with a generator expression instead of a for loop:

Before:

def get_first_even_number(numbers):
    for number in numbers:
        if number % 2 == 0:
            return number
    return None

After

def get_first_even_number(numbers):
    return next((number for number in numbers if number % 2 == 0), None)

My code was a little bit more complicated than this example and I needed time to understand the refactored code.

I really do not understand how does the refactored form “make our code and our intent clearer”. I think I will disable this refactoring as useless in my configuration. What are your opinions?

Then only advantage I see is that the code was converted to a single return statement with a single expression but the considerable cost is the high complexity of the expression and the less common second argument of next().

1 Like

I mean…I get what it is doing, but I’d never do that in real code. It doesn’t make it easier to understand for the average person at all

I can’t see why this would be considered better.

4 Likes

I don’t think it’s better either!

IMO your original code reads well.

If you’re interested in refactoring in a functional style, try this:

>>> def is_even(n):
...     return n % 2 == 0
...
>>> numbers = [1, 13, 15, 17, 22, 25, 24]
>>> next(filter(is_even, numbers), None)
22

The idea is to combine separately testable and reusable components that can be combined into a data pipeline:

  • is_even() is a reusable predicate for solving even/odd problems.
  • filter() is an existing reusable tool for extracting only data that meets some criterion. It works like a WHERE clause in SQL.
  • next() is an existing reusable tool for extracting the first value in a data stream. It works like LIMIT 1 in SQL.
2 Likes