current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 2 down vote favorite

I am new to Python and can't quite make my if-cases any shorter. Any ideas how to do that?

import csv
fileheader = csv.reader(open("test.csv"), delimiter=",")

# Defines the header of the file opened
header = fileheader.next()

# Loop into the file
for fields in fileheader:

# For each header of the file, it does the following :
# 1/ Detect if the header exists in the file
# 2/ If yes and if the cell contains datas, it's added to the payload that will be used for an HTTP POST request.
# 3/ If not, it doesn't add the cell to the payload.

    if 'url_slug' in header:
        url_slug_index = header.index('url_slug')
        if fields[url_slug_index] == "":
            print "url_slug not defined for %s" % fields[url_index]
        else:
            payload['seo_page[path]'] = fields[url_slug_index]

    if 'keyword' in header:
        keyword_index = header.index('keyword')
        if fields[keyword_index] == "":
            print "keyword not defined for %s" % fields[url_index]
        else:
            payload['seo_page[keyword]'] = fields[keyword_index]

@Nick Burns : Perfect, it made the trick thank you very much ! I am very new to programming and simple reflex as creating custom functions is not here yet...

@tobiak_k : Thanks for the tip! Indeed the name is odd, Ill change it on my script. Need to get used to conventions :)

share|improve this question
add comment

2 Answers

up vote 1 down vote accepted

I would consider wrapping the conditions up inside a function:

def is_header(pattern):
    if pattern in header:
        pattern_index = header.index(pattern)
        if fields[pattern_index]:
            pattern_key = "path" if pattern == "url_slug" else "keyword"
            payload['seo_page[{}]'.format(pattern_key)] = fields[pattern_index]            
        else:
            print("{} not defined for {}".format(pattern, fields[pattern_index])

Because the tests are essentially the same, you could create a common function and pass in the pattern to test against (url_slug or keyword). There is some extra hocus pocus to get pattern_key to be the right thing in order to point to the right key in your seo_page dict (I assume it is a dict?).

share|improve this answer
add comment
up vote 0 down vote

Instead of fileheader = csv.reader(open("test.csv"), delimiter=",") you should do

with open("test.csv") as f:
    fileheader = csv.reader(f, delimiter=",")

to ensure that the file is property closed at the end of the script.

Also, why is the reader holding the entire CSV file named fileheader? This seems odd.

share|improve this answer
add comment

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.