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<name>[a-zA-Z]+)\s(?P<link>\S+)$')
get_pattern = re.compile('^get\s(?P<name>[a-zA-Z]+)\s?((?P<minutes>\d+)m)?\s?((?P<seconds>\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))
            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'])
        sleep_time = calculate_time(match)
        post_message(links[name], message['channel'])

def handle_messages_read(messages):
    if messages and len(messages) > 0:
        for message in messages:
            if message and 'text' in message:

def post_message(message, channel='#general'):

if __name__ == '__main__':
    if slack_client.rtm_connect():
        print('KlausBot started and running')
        while True:
            messages = slack_client.rtm_read()
            handle_message_thread = Thread(target=handle_messages_read, args=(messages,))
        print('Could not connect')

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

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


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:


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.


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.