Analyzing records of London Underground outages

Question

You have a file that describes historic data of the London Underground showing dates when stations have been unavailable. The file contains these fields, separated by a pipe character:

• Date: Format of this is mm-dd-yyyy.
• Line: Each line has a unique name. For example, “Northern”.
• Station: Each station has a unique name. For example, “Angel”.
• Notes: Describes the outage

Here is an example of one entry in the file, which shows that the Northern Line was not operating at Bank on 23 August 2019 due to a function at London Bridge.

08-23-2019|Northern|Bank |No service on Bank branch due to birthday booking at London Bridge station.

Write a script that solves the following problems:

1. Display each station that was affected by an outage on 10 July 2019.

2. Between 10 July 2017 and 14 November 2017, which lines had outages and what was the number of stations affected?

I was able to complete both problems but the review of the work said it was done poorly.

# I created a test input file in the same folder as a proxy input.

import datetime  # import built in datetime module for problem 2
with open('input.txt') as file:  # read the input file into a list of lines
    file_input = file.read().splitlines()

# split the file input on the pipe
LonUndOutages = []
for var in file_input:
    LonUndOutages.append(var.split("|"))

# creating empty lists for the answers for Problem 2
Prob1Output = ""
LonUndStations = []
LonUndLines = []

# solving problem 1 and prep for the answers for Problem 2
print("Problem 1:")
for line in LonUndOutages:
    if line[0] == "07-10-2019":
        Prob1Output = Prob1Output + line[2] + "was affected by an outage on July 10, 2019\n"
    line[0] = datetime.datetime.strptime(line[0], '%m-%d-%Y')  # convert the date in line[0] to a datetime object
    if datetime.datetime(2017,7,10) < line[0] < datetime.datetime(2017,11,14):
        # the above line checks if line[0] is in the time range july 10 2017 to nov 14, 2017
        if line[1] not in LonUndLines:  # ensures LonUndLines doesnt have duplicates
            LonUndLines.append(line[1])  # adds each line that had an outage to a list
        if line[2] not in LonUndStations:  # ensures LonUndStations doesnt have duplicates
            LonUndStations.append(line[2])  # adds each station that had an outage to a list


if not Prob1Output:
    print("There were no stations affected by an outage on July 10, 2019")
else:
    print(Prob1Output)
print()
print("Problem 2:")
print("The following lines had outages:")
print(*LonUndLines, sep=', ')
print("During the period between July 10, 2017 and November 15, 2017 there were " + str(len(LonUndStations))
      + " station(s) affected")

Answer 1

That's a fun problem.

Don't use more memory than you have to

with open('input.txt') as file:  # read the input file into a list of lines
    file_input = file.read().splitlines()
# ...
for var in file_input:
    LonUndOutages.append(var.split("|"))

shouldn't be necessary. Rather than calling file.read(), iterate on the file object itself. It will yield lines that you can split on directly.

Naming conventions

LonUndOutages

becomes

lon_und_outages

and similarly for your other variables.

Unpack your line

Assign variables to the results of your split, like so:

date, line, station, notes = outage

Note that there's a nomenclature collision, because your spec describes one column as line, so call each file line a record or an outage.

Separate output from logic

        Prob1Output = Prob1Output + line[2] + "was affected by an outage on July 10, 2019\n"

You're storing output early for two purposes: to eventually show that output, and to know whether there were stations affected. This dual-purpose code is generally not advisable. Store the confirmed outages in a separate list, or yield them from a generator function, to be consumed and formatted by an output function that has no logic at all.

General

Run a linter that will tell you PEP8 hints, or use an IDE that will do the same. Make functions to house your logic.

Example

from collections import namedtuple
from datetime import date, datetime
from typing import Iterable, Set

Record = namedtuple('Record', ('date', 'line', 'station', 'notes'))


def read_file(fn: str) -> Iterable[Record]:
    with open(fn) as file:
        for record in file:
            date_str, *others = record.split('|')
            record_date = datetime.strptime(date_str, '%m-%d-%Y').date()
            yield Record(record_date, *others)


def stations_on_date(records: Iterable[Record], record_date: date) -> Set[str]:
    return set(r.station for r in records if r.date == record_date)


def lines_and_stations_between_dates(
    records: Iterable[Record],
    start: date,
    end: date,
) -> (Set[str], Set[str]):
    matching = [r for r in records if start <= r.date <= end]
    lines = set(r.line for r in matching)
    stations = set(r.station for r in matching)
    return lines, stations


def print_iter(to_print: Iterable[str]):
    msg = ', '.join(to_print)
    print(msg or 'none')


def main():
    records = tuple(read_file('input.txt'))

    date1 = date(2019, 7, 10)
    print(f'Stations affected by an outage on {date1}:')
    stations1 = stations_on_date(records, date1)
    print_iter(stations1)

    start2 = date(2017, 7, 10)
    end2 = date(2017, 11, 14)
    range_desc = f'between {start2} and {end2}'
    lines2, stations2 = lines_and_stations_between_dates(records, start2, end2)

    print(f'Lines with outages {range_desc}:')
    print_iter(lines2)

    print(f'Number of stations affected {range_desc}:')
    print(len(stations2))


if __name__ == '__main__':
    main()

Note:

• Output and logic are separated
• Use a namedtuple for better structure
• Hold onto a date, not a datetime
• Use set to avoid duplicates in the output

Answer 2

Memory Usage

When reading from the file, you can assign create the list there instead of reading lines, splitting them, and appending to the list later in the program. It can be as easy as this

with open('input.txt') as file:
    outages = [line.split("|") for line in file]

Variable Naming

PEP-8 guidelines say that variables should be in snake_case. The only time you should use PascalCase is when dealing with classes. Since you're dealing with specific outages in London, you can simply remove London from the variable names, making them shorter.

LonUndOutages -> outages
LonUndStations -> stations
LonUndLines -> lines

Creating Lists with Loops

Like shown above, when you have

my_list = []
for item in other_list:
    my_list.append(item)

This can be shortened to one line

my_list = [item for item in other_list]

Simplifying if/else

This

if not Prob1Output:
    print("There were no stations affected by an outage on July 10, 2019")
else:
    print(Prob1Output)

can become this

print("There were no stations affected by an outage on July 10, 2019" if not Prob1Output else Prob1Output)

Looks neater and less chunky.

Formatting Strings

You can directly include variables in your strings without having to type cast them, using f"". From this

print("During the period between July 10, 2017 and November 15, 2017 there were " + str(len(LonUndStations))
      + " station(s) affected")

to this

print(f"During the period between July 10, 2017 and November 15, 2017 there were {len(stations)} station(s) affected.")

Too Many Comments

Comments should mainly be used to describe code that may be hard to comprehend, like an algorithm, or why you chose to do something. Comments like this

import datetime  # import built in datetime module for problem 2
with open('input.txt') as file:  # read the input file into a list of lines
    file_input = file.read().splitlines()

# split the file input on the pipe
LonUndOutages = []
for var in file_input:
    LonUndOutages.append(var.split("|"))

Anyone reading the code and see what it does, without the comments. Comments like these can decrease the readability of your code, as readers see more bulk when reading your code.