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

Python Email Program

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

Problem

I've written this email program in Python, and I would like to get some feedback on it. (i.e. closing the server twice after the raw input, etc.)

```
import console
import socket
import smtplib
from getpass import getpass

user = raw_input('What is your name? ').title()
host_name = socket.gethostname()
print '\nThis program is being run by %s on machine name %s' % (user, host_name)

# here we collect the login information for the email
# we also collect the email provider to automatically change the server
email_provider = raw_input('Gmail, AOL, Yahoo! or Comcast? ').title()
email_user = raw_input('Type in your full email username. ') # collect the user name
email_pwd = getpass('Type in your email password. ') # collect the users password

if email_provider in ('Gmail', 'Google'):
smtpserver = smtplib.SMTP("smtp.gmail.com",587)
if email_provider in ('Aol', 'AOL'):
smtpserver = smtplib.SMTP("smtp.aol.com",587)
if email_provider in ('Yahoo', 'Yahoo!'):
smtpserver = smtplib.SMTP("smtp.mail.yahoo.com",587)
if email_provider in ('Comcast'):
smtpserver = smtplib.SMTP("smtp.comcast.net",587)

smtpserver.ehlo()
smtpserver.starttls()
smtpserver.ehlo
smtpserver.login(email_user, email_pwd)

def main():
run_again = 'y'
while run_again == 'y':
# here we are collecting the Sendto email address
# we save the input to a variable named sendto
sendto = raw_input('Email address to send message to: ')
to = sendto
CC = sendto

subj = raw_input('Subject: ')

header = 'To: ' + to + '\n' + 'From: ' + email_user + '\n' + 'Subject: ' + subj +'\n'
print '\nMessage Details:'
print (header)

assignment = raw_input('Message: ')

msg = header + assignment + '\n'
smtpserver.sendmail(email_user, to, msg)
console.hud_alert('Your message has been sent!', 'success', 2.1)
run_again = raw_input('Would you like to send another message? y or n')
if run_again

Solution

-
I would suggest putting all the code in main() except for the imports. Then at the bottom, do

if __name__ == '__main__':
    main(sys.argv)


The advantage of structuring your file this way is that you can run the python interpreter and import your code and play with it. If you break it up into multiple routines, you can run each routine by itself to be sure it does the right thing. As it is now, if you try to import the file, half the code will be run immediately and then main() gets run unconditionally at the bottom, so you can't control it easily in the python interpreter.

-
To translate email_provider to smtp_server, you could do

host = {'GMAIL': "smtp.gmail.com",
        'GOOGLE': "smtp.gmail.com",
        'AOL': "smtp.aol.com",
        'YAHOO': "smtp.mail.yahoo.com",
        'COMCAST': "smtp.comcast.net"}

smtpserver = smtplib.SMTP(host[email_provider.strip('!').upper()], 587)


-
And you might want to handle the case where the user types an unknown email provider.

try:
    smtpserver = smtplib.SMTP(
          host[email_provider.strip('!').upper()], 587)
except KeyError:
    print("Unknown email provider '%s'" % email_provider)
    sys.exit(1)


-
Instead of making the user type 'y' to run again every time, you could do something like this:

while True:
    sendto = raw_input("Target address (or just hit ENTER to quit) > ")
    if sendto == '':
        sys.exit(0)
    to = CC = sendto
    ...

Code Snippets

if __name__ == '__main__':
    main(sys.argv)
host = {'GMAIL': "smtp.gmail.com",
        'GOOGLE': "smtp.gmail.com",
        'AOL': "smtp.aol.com",
        'YAHOO': "smtp.mail.yahoo.com",
        'COMCAST': "smtp.comcast.net"}

smtpserver = smtplib.SMTP(host[email_provider.strip('!').upper()], 587)
try:
    smtpserver = smtplib.SMTP(
          host[email_provider.strip('!').upper()], 587)
except KeyError:
    print("Unknown email provider '%s'" % email_provider)
    sys.exit(1)
while True:
    sendto = raw_input("Target address (or just hit ENTER to quit) > ")
    if sendto == '':
        sys.exit(0)
    to = CC = sendto
    ...

Context

StackExchange Code Review Q#47411, answer score: 5

Revisions (0)

No revisions yet.