7
\$\begingroup\$

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 == 'n': smtpserver.close()
 else: smtpserver.close()
main()
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Apr 16, 2014 at 23:14
\$\endgroup\$
1
  • \$\begingroup\$ if ...: smtpserver.close() else: smtpserver.close() -> smtpserver.close() \$\endgroup\$ Commented Feb 17, 2019 at 14:43

2 Answers 2

5
+50
\$\begingroup\$
  • 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
     ...
    
answered Apr 16, 2014 at 23:43
\$\endgroup\$
3
\$\begingroup\$

Naming

You may be using the smtplib library to help you talk to the SMTP server, but the object returned by smtplib.SMTP() is an SMTP client, not a server. When you call methods on that object, you are asking the client to say EHLO, initiate the TLS handshake, etc. Therefore, from a programming point of view, that object should be called smtpclient, not smtpserver.

Superfluous code

Prompting for the user's name and printing the machine's own hostname is completely irrelevant to the goal of sending e-mail, and in fact you never again refer to user and host_name after the first print statement.

smtpserver.ehlo() calls a method that makes the client initiate the SMTP handshake. However, smtpserver.ehlo a couple of lines further down accomplishes nothing.

The CC variable is never used, and is therefore misleading.

Overall structure

Prompting the user for the service provider and connecting to the SMTP server before main() is ever called is weird. By convention, if you have a function named main(), that's the entry point of your program, and the only code that runs outside the main() function is the code that calls it.

Starting the SMTP session on program startup is the wrong thing to do. If the user takes a long time to compose the mail message, then the server will disconnect the client after an inactivity timeout period has elapsed. Then, when the user finally wants to send the message, an smtplib.SMTPServerDisconnected exception will occur. You don't have any exception handling in your code.

answered Apr 17, 2014 at 7:34
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.