-6
\$\begingroup\$

I have written this code where per entity in feed.entity, if that entity has a field named trip_update and it has a matching route_id then I must iterate through each trip_update.stop_time_update in order to make sure the proper metra transit (using stop_id comparison) is selected. Then, I calculate some data; then iterate through each trip in metraTripIds. Then run a basic if statement to make sure the proper location is returned (which isn't a big deal). I just feel like having 3 for statements within each other can be refactored to be performed faster too. I just don't see how in this situation, would love to hear what everyone else thinks!

for entity in feed.entity:
        if (entity.HasField('trip_update')) and (str(entity.trip_update.trip.route_id) == metraRouteId):
            for stoptime in entity.trip_update.stop_time_update:
                if (str(stoptime.stop_id) == metraStopId):
                    delayedTime = int((int(stoptime.arrival.time) - currentTime) / 60)
                    tripID = str(entity.trip_update.trip.trip_id).split('_')[1][2:]
                    for metraTrip in metraTripIds:
                        if (entity.trip_update.trip.trip_id == metraTrip["trip_id"]):
                            if (metraTrip["trip_headsign"] == metraTripHeadsignInbound):
                                metraLocation = metraInboundView
                            else:
                                metraLocation = metraOutboundView
                    predictionArray.append({'time': delayedTime, 'location': metraLocation, 'transitNumber': tripID, 'key': _ignoreThis})

    return predictionArray
\$\endgroup\$
2
  • \$\begingroup\$ Please fix the indentation. Also retitle the question to summarize the task accomplished by the code. See How to Ask. \$\endgroup\$ Commented Apr 11, 2018 at 21:52
  • \$\begingroup\$ This question is incomplete. To help reviewers give you better answers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. Questions should include a description of what the code does \$\endgroup\$ Commented Apr 11, 2018 at 22:12

1 Answer 1

Reset to default
2
\$\begingroup\$

Some options:

  1. Rather that using

    for foo in bar:
    if something:
        process foo

    you can flip the condition around and terminate early:

    for foo in bar:
    if not something:
        continue
    process foo

    this reduces the nesting level from the number of loops + the number of conditionals to just the number of loops. I believe this is controversial, though. Some devs don't like early termination in general, and it does lean towards cargo cult behaviour - indentation, in itself, does not make code objectively more complex, and terminating early does objectively add a tiny bit of complexity in the form of a continue.

  2. Chunk logically related lines in functions. Often the contents of an if block belong together, so you'd convert

    for cell in table:
    if something:
        preprocess row
        for cell in row:
            if something:
                process cell

    to

    for cell in table:
    if something:
        process_row(row)

def process_row(row):
    preprocess row
    for cell in row:
        if something:
            process_cell(cell)

def process_cell(cell):
    […]
  3. I suspect there is a bug in this code. metraLocation is only set (or changed) the conditionentity.trip_update.trip.trip_id == metraTrip["trip_id"] holds. If that condition does not hold for the first item then I believe the script will crash. If it does hold for the first item but not for one of the subsequent items, then that predictionArray entry will get the value from the previous iteration.
\$\endgroup\$
1
  • \$\begingroup\$ I appreciate the help. In terms of part 3, i initialize metraLocation to an empty string, so it'll return an empty string if all fails. It should/is meant to relate to a metraTrip["trip_id"] though otherwise the program should fail. Thank you! \$\endgroup\$ Commented Apr 11, 2018 at 23:59

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.