patternpythonMinor
Simple slack bot in python
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)
`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:
This:
Can simply be this:
... since
Iterating over 0 length item:
This:
Can simply be this:
... since the for loop will not have anything to do if the length is zero.
Also, I am not sure what this
If guarding against
And if the check is a surrogate length check, then it isn't necessary at all, and the if could be removed.
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.