My bot is fully functional and I would love some feedback. Thanks!

Main loop for keep it running (main.py)

from functions import RunBot, InitPraw

reddit_handler = InitPraw()
doctor_who_subreddit_handler = reddit_handler.subreddit("doctorwho")


Functions that make it work (functions.py)

from datetime import datetime
from time import sleep
from praw import Reddit
from os import environ
from random import randint
import pytz
from sys import stderr

def InitPraw():
  return Reddit(
    client_id = environ['CLIENT_ID'],
    client_secret = environ['CLIENT_SECRET'],
    user_agent="console:doctor-who-bot:v1.0.0 (by u/doctor-who-bot)",
    username = "doctor-who-bot",
    password = environ['PASSWORD']

def LoadQuotes():
  quotes = []
  file = open('quotes.txt', 'r', encoding='utf-8')
  for line in file:
  return quotes

def AlreadyReplied(replies):
  for reply in replies:
    if reply.author == "doctor-who-bot":
      return True
  return False

def GetRandomPositionOfObject(object):
  return randint(0, len(object)-1)

def ReplyRandomQuote(comment):
  quotes = LoadQuotes()
  random_quote_position = GetRandomPositionOfObject(quotes)
  reply = quotes[random_quote_position] + "\n" + "^(I'm a bot and this action was performed automatically)" + "\n" + "\n" + "^(Feedback? Bugs? )[^(Contact the developer)](mailto:marcosmartinezpalermo@gmail.com)" + "\n" + "\n" "[^(Github)](https://github.com/marcosmarp/doctor-who-bot)"
  return quotes[random_quote_position]

def StoreReply(comment, reply):
  amount_of_lines = 0
  with open("replies.txt", "r", encoding='utf-8') as file_object:
    for line in file_object:
      amount_of_lines += 1
  with open("replies.txt", "a", encoding='utf-8') as file_object:
    file_object.write("Reply #" + str(int(amount_of_lines/11 + 1)))
    file_object.write(" Replied comment data:")
    file_object.write("   Author: " + comment.author.name)
    file_object.write("   Link: https://www.reddit.com" + comment.permalink)
    file_object.write("   Post:")
    file_object.write("     Title: " + comment.submission.title)
    file_object.write("     Author: " + comment.submission.author.name)
    file_object.write("     Link: https://www.reddit.com" + comment.submission.permalink)
    file_object.write(" Reply data:")
    file_object.write("   Replied quote: " + reply)

def InformReplyOnScreen(comment, reply):
  now = datetime.now(pytz.timezone('America/Argentina/Buenos_Aires'))
  dt_string = now.strftime("%d/%m/%Y %H:%M:%S")
  print(dt_string + ": replied " + comment.author.name + "'s comment with: ", file=stderr)
  print("   " + reply, file=stderr)

def CheckNewPosts(posts):
  for post in posts:
    for comment in post.comments:
      if hasattr(comment, "body"):
        if "doctor" in comment.body.lower():
          if not AlreadyReplied(comment.replies):
            quote_replied = ReplyRandomQuote(comment)
            InformReplyOnScreen(comment, quote_replied)
            StoreReply(comment, quote_replied)
            sleep(600) # Until I gain karma, minimum time between comments is 10 min

def RunBot(subreddit_handler):

And, to keep it running, I'm using a TMUX session on an AWS EC2 instance, so any comment regarding that is also appreciated.

Thanks again!


1 Answer 1


Function and variable names in Python should follow snake_case.

PEP 8 recommends using 4 spaces per indentation level.


Whenever possible you should avoid manually opening and closing files, you can let a context manager take care of that for you (which is less error-prone):

with open('quotes.txt', 'r', encoding='utf-8') as file:
  # ... everything that accesses file happens in here

# ... everything after accessing file happens here

In Python you can and should usually avoid this pattern of list creation:

some_list = []
for element in some_iterable:

List comprehensions offer a readable and fast way of achieving the same result:

some_list = [element for element in some_iterable]

Even better, if you don't check or modify the elements, you can simply pass the iterable to the list constructor:

some_list = list(some_iterable)

In case of reading a file I would recommend using readlines:

def load_quotes():
    with open('quotes.txt', 'r', encoding='utf-8') as file:
        return file.readlines()


Using built-in any and a generator expression we get this really concise and readable implementation:

def already_replied(replies):
    return any(reply.author == "doctor-who-bot" for reply in replies)

any basically checks if an iterable contains at least one element that is True or truthy. The iterable we are passing to the function is a generator expression .

GetRandomPositionOfObject & ReplyRandomQuote

The random module from the standard library offers the choice function, which covers your exact use case. So instead of

random_quote_position = GetRandomPositionOfObject(quotes)
reply = quotes[random_quote_position] + ...

you can use choice like this

random_quote = random.choice(quotes)
reply = random_quote + ...

# ...

return random_quote

and get rid of GetRandomPositionOfObject altogether.


You're already using context managers here, great! Since they take care of closing the file you do not need to call file_object.close() manually.

We can count the number of elements in an iterable using built-in sum and a generator expression:

number_of_elements = sum(1 for _ in some_iterable)

Using _ as a variable is a convention that signals that the value of the variable is never actually used.

Note: In general we can use built-in len to get the length of an iterable:

number_of_elements = len(some_iterable)

However, this does not work in this case since object of type '_io.TextIOWrapper' has no len(). You could use it together with readlines: amount_of_lines = len(file_object.readlines()), but I'd think this is a waste of memory, since it reads and stores all lines before counting them.

So after applying these refactorings, we get

with open("replies.txt", "r", encoding='utf-8') as file_object:
    amount_of_lines = sum(1 for _ in file_object)

Instead of manual conversion int(amount_of_lines / 11 + 1) you can use the floor-division operator //, which returns an int: amount_of_lines // 11 + 1.

If your intention is to only round up amount_of_lines / 11 you will run into a bug whenever amount_of_lines % 11 == 0. Instead use math.ceil from the standard library: math.ceil(amount_of_lines / 11)

To get rid of explicit string-conversion you can let f-strings conveniently handle this for you:

file_object.write("Reply #" + str(int(amount_of_lines / 11 + 1)))


file_object.write(f"Reply #{math.ceil(amount_of_lines / 11)}")

You can also use f-strings in the other calls to file_object.write() to avoid manual string concatenation and write variables in a readable way.

Of course, same goes for string construction in other functions.


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.