6
\$\begingroup\$

Context: I wrote a simple as an answer to a problem in Unix.SE and it was suggested to me to post it here for review.

A comma separated file is generated by an application. On the first column it contains a last name, on the second a first name and on the tenth column a telephone number. For each row of this csv an vcard entry should be generated.

This was my solution:

#!/usr/bin/python

import csv
import sys

def convert(file):
    reader = csv.reader(open(file, 'rb'))

    for row in reader:
        print 'BEGIN:VCARD'
        print 'VERSION:2.1'
        print 'N:' + row[0] + ';' + row[1]
        print 'FN:' + row[1] + ' ' + row[0]
        print 'TEL;HOME;VOICE:' + row[9]
        print 'END:VCARD'

def main(args):
    if len(args) != 2:
        print "Usage:"
        print args[0] + " filename"
        return

    convert(args[1])

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

Outputting the generated vcard on stdout was fine for me, I just redirected it into an appropriate file.

\$\endgroup\$

5 Answers 5

5
\$\begingroup\$

Read about the with statement and ALWAYS use it with file operations

with open( somefilename, 'rb' ) as source:
    reader = csv.reader( source )
    for row in reader:
       etc.

That guarantees that the file is closed and all OS resources released.

\$\endgroup\$
3
\$\begingroup\$

Use sys.exit to indicate success or failure of the program.

file is a builtin python function, its best to avoid naming your variables the same thing.

for row in reader:
    print 'BEGIN:VCARD'
    print 'VERSION:2.1'
    print 'N:' + row[0] + ';' + row[1]
    print 'FN:' + row[1] + ' ' + row[0]
    print 'TEL;HOME;VOICE:' + row[9]
    print 'END:VCARD'

I'd recommend putting in something like

foo = row[0]

and then printing foo to make it clearer what the different values are.

\$\endgroup\$
3
\$\begingroup\$

Even for something this trivial, use argparse.

import argparse

parser = argparse.ArgumentParser(description='Convert a CSV or whatever.')
parser.add_argument('filenames',  type=argparse.FileType('rb'), nargs='+',
                   help='a file to convert')

args = parser.parse_args()
for sourcefile in args.filenames:
    convert( sourcefile )
\$\endgroup\$
2
  • \$\begingroup\$ (meta) Is it proper here to update the original question with the reviews applied? \$\endgroup\$
    – Vitor Py
    Jul 18, 2011 at 21:31
  • 1
    \$\begingroup\$ You could, but you'd have to leave the original code in place. However. I don't think that would accomplish much. You can beat a single example "to death" through over-analysis. What do you think you might learn from posting the revised code? \$\endgroup\$
    – S.Lott
    Jul 18, 2011 at 22:11
1
\$\begingroup\$

Python Manual mentions that you should open a file with newline='' csv

Thus:

with open( somefilename, 'rb', newline='') as source:
   etc
\$\endgroup\$
0
\$\begingroup\$

Whenever you assemble text for future computer interpretation, whether by string concatenation or interpolation, alarm bells should go off: some escaping mechanism is essential! Failure to do so would expose you to a class of bugs akin to XSS and SQL injection.

According to the vCard version 2.1 specification, semicolons must be escaped with a backslash. The 3.0 specification mandates the use of escape sequences \\, \;, \,, and \n as well.

\$\endgroup\$

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