When to create a generator or return simply a list in Python? [closed]

If you are going to need the whole list for some reason, then make a list. But if you only need one item at a time, use a generator. The reasons for preferring generators are (i) you only need the memory for one item at a time; (ii) if the items are coming from a file or network connection or database then generators give you some parallelism because you can be processing the first item without having to wait for all items to arrive; (iii) generators can handle infinite streams of items.

There is no special convention for naming generators in Python. It is best to name a function after the things it generates.

Instead of:

[yearnow - i for i in range(4)]

write:

range(yearnow, yearnow - 4, -1)

The duplicated code could be avoided like this:

if year:
    year_range = (year,)
else:
    year_now = datetime.now().year
    year_range = range(year_now, year_now - 4, -1)
for y in year_range:
    if self.records:
        yield self.records[utils.dateracetype_to_DBcolumn(y, str(race_type))]
    else:
        yield None

(But I don't recommend implementing it like this, as I'll explain below.)

It is conventional to use None as the default value for an argument like year. A default like year=0 might be confused with a legitimate value for year. For example, in astronomical year numbering there is a year zero, and how would you pass this to your function?

Instead of yielding an exceptional value (here None) when there are no results to generate, just generate no results. The problem with exceptional values is that the caller might forget to check for them, whereas it is usually straightforward to handle the case of no results.

I don't think it's a good idea to use datetime.now().year for the case where the caller doesn't specify a year. The problem is that close to midnight on 31st December, there is a race: the current year might be 2018 just before the call to list_records happens, and 2019 just after. What should list_records return in this case? The Zen of Python says,

In the face of ambiguity, refuse the temptation to guess.

so it would be better to avoid this problem by requiring the caller to specify the year.

The function does two different things: it generates the record for a year (if a year was specified), or it generates records from the last four years (if no year was specified). The single responsibility principle suggests that these two things should be implemented by two functions:

def year_record(self, race_type, year):
    """Return the record for race_type and year.
    Raise IndexError if there are no records.

    """
    return self.records[utils.dateracetype_to_DBcolumn(year, str(race_type))]

def recent_records(self, race_type, year, n=4):
    """Generate the records for race_type for the n years up to (and
    including) year, in reverse order.

    """
    for y in range(year, year - n, -1):
        try:
            yield self.year_record(race_type, y)
        except IndexError:
            pass

If the caller needs to know whether there was a record for each year, then I recommend generating the necessary data explicitly, like this:

def recent_records(self, race_type, year, n=4):
    """Generate the records for race_type for the n years up to (and
    including) year, in reverse order. For each year, generate a tuple
    (year, have_record, record) where have_record is True if there
    is a record for the given year, or False if not.

    """
    for y in range(year, year - n, -1):
        try:
            yield year, True, self.year_record(race_type, y)
        except IndexError:
            yield year, False, None

Then the caller can write:

for year, have_record, record in self.recent_records(...):
    if have_record:
        # ...