ImprSimple chat bot written in python

Question

I'd like to know if I can improve the performance of my recent bot, and maybe some design patterns too.

So far, it's warning a user if it's using some bad words (which are parsed from bad_words.txt) and it's entertaining the users by posting jokes from jokes.txt.

from selenium import webdriver
import time
import random


def login_to_rst(base_url, username, password):
    xpaths = {
        'usernameTxtField': "//input[@name='auth']",
        'passwordTxtField': "//input[@name='password']",
        'submitButton': "//*[@id='ipsLayout_mainArea']/div/div[2]/div/form/li[5]/div/button"
    }
    FIREFOX_DRIVER.get(base_url)
    FIREFOX_DRIVER.find_element_by_xpath(xpaths['usernameTxtField']).clear()
    FIREFOX_DRIVER.find_element_by_xpath(xpaths['usernameTxtField']).send_keys(username)
    FIREFOX_DRIVER.find_element_by_xpath(xpaths['passwordTxtField']).clear()
    FIREFOX_DRIVER.find_element_by_xpath(xpaths['passwordTxtField']).send_keys(password)
    FIREFOX_DRIVER.find_element_by_xpath(xpaths['submitButton']).click()
    FIREFOX_DRIVER.get(base_url)


def write_to_chat(chat_url):
    FIREFOX_DRIVER.get(chat_url)

    bad_words = [line.rstrip('\n') for line in open('bad_words.txt')]

    time.sleep(5)
    message = "Please use a decent language !"
    xpaths = {
        'textArea': "//*[@id='ipsTabs_elChatTabs_chatroom_panel']/div[1]/div[1]/textarea",
        'submitMessage': "//*[@id='ipsTabs_elChatTabs_chatroom_panel']/div[1]/div[1]/div[3]/button"
    }

    parsed_messages, rst_messages_list = [], []
    keep_running = True
    while keep_running:
        divs = FIREFOX_DRIVER.find_elements_by_xpath('//ul[@class="ipsList_reset cChatContainer"]/li/div')
        for div in divs:
            rst_messages_list = []
            if div.id not in parsed_messages:
                rst_messages_list.append(div.text)
                print(rst_messages_list)
                parsed_messages.append(div.id)

                for unique_message in rst_messages_list:
                    for word in bad_words:
                        if word in unique_message:
                            FIREFOX_DRIVER.find_element_by_xpath(xpaths['textArea']).clear()
                            FIREFOX_DRIVER.find_element_by_xpath(xpaths['textArea']).send_keys(message)
                            FIREFOX_DRIVER.find_element_by_xpath(xpaths['submitMessage']).click()

                    if '/message1' in unique_message or '/Message1' in unique_message:
                        FIREFOX_DRIVER.find_element_by_xpath(xpaths['textArea']).clear()
                        FIREFOX_DRIVER.find_element_by_xpath(xpaths['textArea']).send_keys("text to be sent")
                        FIREFOX_DRIVER.find_element_by_xpath(xpaths['submitMessage']).click()
                    if '/message2' in unique_message or '/Message2' in unique_message:
                        FIREFOX_DRIVER.find_element_by_xpath(xpaths['textArea']).clear()
                        FIREFOX_DRIVER.find_element_by_xpath(xpaths['textArea']).send_keys("2016 © me")
                        FIREFOX_DRIVER.find_element_by_xpath(xpaths['submitMessage']).click()
                    if '/message3' in unique_message:
                        line = random.choice(open('jokes.txt').readlines())
                        FIREFOX_DRIVER.find_element_by_xpath(xpaths['textArea']).clear()
                        FIREFOX_DRIVER.find_element_by_xpath(xpaths['textArea']).send_keys(line)
                        FIREFOX_DRIVER.find_element_by_xpath(xpaths['submitMessage']).click()


if __name__ == "__main__":
    FIREFOX_DRIVER = webdriver.Firefox()

    login_to_rst('https://link-login', 'user', 'pass')
    write_to_chat('https://link-final-url')

I accept any constructive critiques as long as they are followed by a rational explanation.

Answer 1

Avoid using open when you can. with is a keyword for using a context manager, which will automatically ensure that files are always properly closed, which you don't ensure when you just call open in a list comprehension.

with open('bad_words.txt') as f:
    bad_words = [line.rstrip('\n') for line in f]

keep_running is always True, so why not just use while True.

To reduce nesting levels, I'd flip the condition from this:

rst_messages_list = []
if div.id not in parsed_messages:
    rst_messages_list.append(div.text)

to this

rst_messages_list = []
if div.id in parsed_messages:
    continue

rst_messages_list.append(div.text)

continue moves onto the next iteration, so it is effectively the same as before except that your code isn't as deeply nested.

When testing unique_message, you should really make it lowercase for ease of testing. That way you can match regardless of case. Also when you're testing if any word in bad_words is in unique_message, there's a Python function that does that, called any:

for unique_message in rst_messages_list:
    unique_message = unique_message.lower()
    if any(word in unique_message for word in bad_words):

This does the same as you had before, but now the if and for statements are combined in one line. The one main difference, is that your old code would message the user repeatedly if they used multiple bad words, which I assume was not the intention.

You repeatedly call these lines:

FIREFOX_DRIVER.find_element_by_xpath(xpaths['textArea']).clear()
FIREFOX_DRIVER.find_element_by_xpath(xpaths['textArea']).send_keys(message)
FIREFOX_DRIVER.find_element_by_xpath(xpaths['submitMessage']).click()

Make this a function:

def send_message(message, xpaths):
    FIREFOX_DRIVER.find_element_by_xpath(xpaths['textArea']).clear()
    FIREFOX_DRIVER.find_element_by_xpath(xpaths['textArea']).send_keys(message)
    FIREFOX_DRIVER.find_element_by_xpath(xpaths['submitMessage']).click()

That reduces your code drastically:

if any(word in unique_message for word in bad_words):
    send_message(message, xpaths)
elif '/message1' in unique_message:
    send_message("text to be sent", xpaths)
elif '/message2' in unique_message:
    send_message("2016 © me", xpaths)
elif '/message3' in unique_message:
    line = random.choice(open('jokes.txt').readlines())
    send_message(line, xpaths)

Worth noting that since these are all if commands, you might send every one of these messages, for a message like "/message1 /message2 /message3 smeg" (assuming that smeg is on your list of bad_words). You should all but the first one elifs, so that they're mutually exclusive and only one (or none) will run.

Last point, unless calling webdriver.Firefox() is a lot of work for the script, I'd put that at the top instead of in if __name__ == "__main__", as it's easier to spot when it's near the top of the page, after imports. That's the convention for where constants should go.

FIREFOX_DRIVER = webdriver.Firefox()