1
\$\begingroup\$

This is a plugin for hangoutsbot/hangoutsbot that listens for words people say and then gives them an ETA and a link with directions.

Happy to hear about any optimisations that could be made.

import logging
import hangups
import plugins
import asyncio

import googlemaps

logger = logging.getLogger(__name__)

from textblob import TextBlob
from urllib.parse import quote

def _initialise(bot):
  plugins.register_handler(_handle_message, type="message")

def _handle_message(bot, event, command):
  raw_text = " ".join(event.text.split())
  directions = ["how", "long", "take", "to", "from"]
  if all(x in raw_text.lower() for x in directions):
      yield from _getdirections(bot, event, raw_text, directions)

@asyncio.coroutine
def _getdirections(bot, event, text, type):
  logger.info("Directions from text: " + text)
  try:
      mapskey = bot.get_config_option("maps_api_key")
  except:
      logger.error("Something went wrong getting the API key. Check it and reload.")
      return
  if not mapskey.startswith("AIza"):
      logger.error("Your API key is wrong, apparently. Check it and reload.")
      return

  bicycling = ["by bicycling","via bicycling", "by cycling","via cycling", "by bike","via bike", "a bicycle", "to cycle"]
  walking = ["on foot", "by walking","via walking", "to walk", "by foot","via foot"]
  transit = ["by public transport","via public transport"]
  train = ["by train","via train"]
  bus = ["by bus","via bus"]
  subway = ["by subway", "on the subway", "by the subway", "via subway", "via the subway"]
  tram = ["via tram", "via light rail","by tram", "by light rail","on the tram", "on the light rail"]

  try:
      regionbias = bot.get_config_option("directions_geobias")
  except:
      regionbias = ""

  routeMode = "driving"
  transitMode = ""

  logger.info("text:" + text)
  text = TextBlob(text)
  for s in text.sentences:
    if any(x in s for x in transit):
        routeMode = "transit"
        for f in transit:
            s = s.replace(f, "")
    elif any(x in s for x in bicycling):
        routeMode = "bicycling"
        for f in bicycling:
            s = s.replace(f, "")
    elif any(x in s for x in walking):
        routeMode = "walking"
        for f in walking:
            s = s.replace(f, "")
    elif any(x in s for x in train):
        routeMode = "transit"
        transitMode = "train"
        for f in train:
            s = s.replace(f, "")
    elif any(x in s for x in bus):
        routeMode = "transit"
        transitMode = "bus"
        for f in bus:
            s = s.replace(f, "")
    elif any(x in s for x in subway):
        routeMode = "transit"
        transitMode = "subway"
        for f in subway:
            s = s.replace(f, "")
    elif any(x in s for x in tram):
        routeMode = "transit"
        transitMode = "tram"
        for f in tram:
            s = s.replace(f, "")

      logger.info(s)
      if all(x in s.lower() for x in type):
          dFrom = s.lower().words.index(type[-1])
          dTo = [i for i, x in enumerate(s.lower().words) if x == type[-2]][-1]

          if dFrom + 1 < dTo:
              origin = " ".join(s.words[dFrom + 1:dTo])
              destination = " ".join(s.words[dTo + 1:])
          elif dTo + 1 < dFrom:
              destination = " ".join(s.words[dTo + 1:dFrom])
              origin = " ".join(s.words[dFrom + 1:])

          gmaps = googlemaps.Client(key=mapskey)

          dirs = gmaps.directions(origin, destination, mode=routeMode, region=regionbias, transit_mode=transitMode)

          logger.info("origin/destination/mode/region/transit_mode:" + "/" + origin + "/"  + destination + "/" + routeMode + "/" + regionbias + "/" + transitMode)

          try:
              dirs1 = dirs[0]
              dirlegs = dirs1["legs"]
              dirleg = dirlegs[0]
              duration = dirleg["duration"]
              time = duration["text"]
              startAddr = dirleg["start_address"]
              endAddr = dirleg["end_address"]
              mapsUrl = "https://www.google.com/maps?f=d&saddr=" + quote(startAddr) + "&daddr=" + quote(endAddr)
              routeUrlParams = {"walking":"w","transit":"r","bicycling":"b"}
              logger.info(dirs1)
              logger.info(dirleg)
              logger.info(dirs)
              if routeMode: mapsUrl = mapsUrl + "&dirflg=" + routeUrlParams[routeMode]
              yield from bot.coro_send_message(event.conv, "Looks like it'll take you " + time + " to get from " + startAddr + " to " + endAddr + '. [<a href="' + mapsUrl + '" >maps</a>]')
          except IndexError:
              logger.error(dirs)
\$\endgroup\$
2
\$\begingroup\$

Long else if chains of repeated instructions are a bad idea, if you ever see them then you should immediately re-think the code, perhaps you could refactor it to work better. In this case, you definitely can. Here's what you're doing for bicycling:

elif any(x in s for x in bicycling):
    routeMode = "bicycling"
    for f in bicycling:
        s = s.replace(f, "")

But that's the same paradigm for all modes of transport:

elif any(x in s for x in transport_name):
    routeMode = "transport_name"
    for f in transport_name:
        s = s.replace(f, "")

Using the above code you could do all of them. Obviously you don't want to set routeMode to just "transport_name", but there's an easy fix to get an accessible string. You should change your data from being multiple lists to be a dictionary of lists. This way you'll have string keys paired with your list values:

TRANSPORT = {
              "bicycling": ["by bicycling", "via bicycling", "by cycling",
                            "via cycling", "by bike","via bike",
                            "a bicycle", "to cycle"],
              "walking": ["on foot", "by walking", "via walking",
                          "to walk", "by foot","via foot"],
             }            

And so on. Now you can loop over all the keys and lists in transport like so:

for sentence in text.sentences:
    for mode, phrases in TRANSPORT.items():
        if any(phrase in sentence for phrase in phrases):
            routeMode = mode
            for phrase in phrases:
                sentence = sentence.replace(phrase, "")
            break

Looping over TRANSPORT.items() allows you to get the key and the value from the dictionary simultaneously. So mode will be set as bicycling and phrases will be set as the list of bicycling strings. I also renamed some of your variables to be clearer. sentence and phrase are more readable than s and f. I also named TRANSPORT in UPPER_SNAKE_CASE to demonstrate that it's a constant value.

In addition, I would recommend using a regular expression instead of a loop with replace. You could build a regular expression using these lists with just | separating each phrase and passing them to re.sub. This would mean that it'd match any occurrences of any of the phrases and replace them with an empty string because | is like an or in regular expressions. The useful part is that it can replace all possible phrases with just one call.

if any(phrase in sentence for phrase in phrases):
    routeMode = mode
    pattern = '|'.join(phrases)
    sentence = re.sub(pattern, '', sentence)
    break

(don't forget to import re for this to work)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.