Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm tasked with getting emails from a .csv file and using them to submit a form. I am using the csv and mechanize Python libraries to achieve this.

import re
import mechanize
import csv

def auto_email(email):
    br = mechanize.Browser()
    br.open("URL")
    br.select_form(name="vote_session")
    br['email_add'] = '%s' % (email) #email address
    br.submit()

def csv_emails():
    ifile  = open('emails.csv', "rb")       
    reader = csv.reader(ifile)
    rownum = 1
    for row in reader:
        auto_email(row[0])
        print "%d - %s processed" %(rownum, row[0])
        rownum += 1
    print 'List processed.  You are done.'
    ifile.close()

print csv_emails()

The code works, but I am very much a beginner in Python.

I was wondering whether I have any inefficiencies that you can help me get rid of and optimize the script?

share|improve this question

2 Answers 2

up vote 6 down vote accepted

I would suggest to use

with open('emails.csv', "rb") as ifile

see here for details: http://www.python.org/dev/peps/pep-0343/ in this case you don't need to do ifile.close at the end

instead of incrementing rownum by yourself, you can use

for row, rownum in enumerate(reader):

I don't see the reason to do this

br['email_add'] = '%s' % (email) #email address

instead of

br['email_add'] = email
share|improve this answer
    
email comes from a row returned by csv.reader so it's always a string. –  Gareth Rees Jan 15 '13 at 17:03
    
so in this case br['email_add'] = email is enough. I edited my answer –  RomanI Jan 15 '13 at 17:04
    
Thanks very much guys –  Kelvin Jayanoris Jan 15 '13 at 19:45

print csv_emails() doesn't make sense. Since csv_emails() returns no value, there is nothing to print.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.