HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Simple slack bot in python

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simpleslackpythonbot

Problem

I wrote a simple slack bot, which is able to save links (command: "save name link") and print them (command: "get name"). Optionally, a delay in minutes and seconds can be specified (command: "get name XmXs"). I would appreciate any feedback.

`import os
import re
from time import sleep
from threading import Thread
from slackclient import SlackClient

BOT_NAME = 'klausbot'
BOT_ID = ''
slack_client = SlackClient(os.environ['SLACK_BOT_TOKEN'])

links = {}
save_pattern = re.compile('^save\s(?P[a-zA-Z]+)\s(?P\S+)$')
get_pattern = re.compile('^get\s(?P[a-zA-Z]+)\s?((?P\d+)m)?\s?((?P\d+)s)?$')

def get_bot_id():
api_call = slack_client.api_call('users.list')
if api_call.get('ok'):
users = api_call.get('members')
for user in users:
if 'name' in user and user.get('name') == BOT_NAME:
global BOT_ID
BOT_ID = ''.format(user.get('id'))

def parse_message(message):
text = message['text']
if text.startswith(BOT_ID):
text = text.split(BOT_ID)[1].strip().lower()
if save_pattern.match(text):
handle_save_command(message, save_pattern.match(text))
elif get_pattern.match(text):
handle_get_command(message, get_pattern.match(text))
else:
post_message('Command not found', message['channel'])

def handle_save_command(message, match):
name = match.group('name')
link = match.group('link')
links[name] = link
response = '{} saved to {}'.format(link, name)
post_message(response, message['channel'])

def calculate_time(match):
minutes = match.group('minutes') or 0
seconds = match.group('seconds') or 0
return int(minutes) * 60 + int(seconds)

def handle_get_command(message, match):
name = match.group('name')
if name not in links:
error_message = '{} not found'.format(name)
post_message(error_message, message['channel'])
else:
sleep_time = calculate_time(match)
sleep(sleep_time)

Solution

In general this code looks pretty good. I do have a couple of small pythonism comments:

dict.get() returns None if key not present:

This:

if 'name' in user and user.get('name') == BOT_NAME:


Can simply be this:

if user.get('name') == BOT_NAME:


... since dict.get() will return None if the key is not present and I presume BOT_NAME will never be None.

Iterating over 0 length item:

This:

if messages and len(messages) > 0:
    for message in messages:


Can simply be this:

if messages:
    for message in messages:


... since the for loop will not have anything to do if the length is zero.

Also, I am not sure what this if is testing for:

if messages:
    for message in messages:


If guarding against None, then it would be more pythonic as:

if messages is not None:
    for message in messages:


And if the check is a surrogate length check, then it isn't necessary at all, and the if could be removed.

Code Snippets

if 'name' in user and user.get('name') == BOT_NAME:
if user.get('name') == BOT_NAME:
if messages and len(messages) > 0:
    for message in messages:
if messages:
    for message in messages:
if messages:
    for message in messages:

Context

StackExchange Code Review Q#157566, answer score: 5

Revisions (0)

No revisions yet.