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

Mailinator automatic checker for new e-mails

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

Problem

I'm making a little script to run on the background and notify me when I received a new email in a mailinator inbox. I want to eventually daemonise it, but for now it just runs like a regular script.

Could you give me some pointers about the code itself, the code structure and so on? I'm not sure if I should have my functions in a particular order, they're quite randomly scattered around the script at the moment.

#!/usr/bin/env python3
#encoding: utf-8

import time
import argparse
import requests

from IPython import embed as qq

try:
    from apikey import API
except ImportError:
    API = input('Please provide your API key ')

CHECKING_FREQ = 30

def parse_arguments():
    parser = argparse.ArgumentParser(description="Mailinator inbox checker")
    parser.add_argument('inbox', nargs=1, help='The inbox that you want to check')
    return parser.parse_args()

def is_new(emails):
    # Here I could probably use any() instead of next()
    # but I'm thinking I might want the email itself later on
    return next((email for email in emails
                 if email['seconds_ago'] <= CHECKING_FREQ),
                False)

def check_inbox(inbox):
    inbox_page = get_inbox(inbox)
    return inbox_page['messages']

def wait_email(inbox):
    while 1:
        emails = check_inbox(inbox)
        if is_new(emails):
            print('New email received.')
        time.sleep(CHECKING_FREQ)

def get_inbox(inbox):
    return requests.get('https://api.mailinator.com/api/inbox?'
                        'to={inbox}&token={api}'.format(
                            inbox=inbox,
                            api=API)).json()

def start_daemon():
    ...

def main():
    args = parse_arguments()
    start_daemon()
    wait_email(args.inbox[0])

if __name__ == '__main__':
    main()

Solution

Overall it looks great, but here are a couple of thoughts:

-
print('New email received.'): maybe print a snippet form the message or just the time stamp so you have some idea on when the new email arrived (instead of just having a stream of same strings stating "New email received.").

-
I'd consider moving the inbox operations into its own class. Its __init__ could be __init__(inbox, api): .... CHECKING_FREQ could be a keyword argument to this, defaulting to 30, but configurable through the input params when running the script. The class could have a fetch_inbox (merging your get_inbox and check_inbox). And finally the has_new_items (just rename your is_new method).

-
Your wait_email is what I'd call check_inbox because for checking the inbox you first have to fetch the inbox and then check the message timestamps on what was returned.

-
I feel someone might have a good suggestion on how to do that infinite loop, but I can't think of a good way right now. All I know is it feels a little odd (or maybe it's fine :).

Hopefully this helps.

Context

StackExchange Code Review Q#73948, answer score: 2

Revisions (0)

No revisions yet.