Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

It can be a good habit to move the part of your program doing things (by opposition to the part of your program defining things) behind an if __name__ == "__main__" if __name__ == "__main__" guard.

It can be a good habit to move the part of your program doing things (by opposition to the part of your program defining things) behind an if __name__ == "__main__" guard.

It can be a good habit to move the part of your program doing things (by opposition to the part of your program defining things) behind an if __name__ == "__main__" guard.

Source Link
SylvainD
  • 29.7k
  • 1
  • 49
  • 93

Style

Python has a style guide called PEP8. Among many other great things, it gives guidelines about spacing that you do not follow. Indeed, your spacing seems to be quite inconsistent. You'll find tools such as pep8 to check your compliancy to PEP8 and other tools such as ``autopep8 to fix your code automatically.

It can be a good habit to move the part of your program doing things (by opposition to the part of your program defining things) behind an if __name__ == "__main__" guard.

You can also use tools such as pylint to check your code. Among other things, Python naming convention are now followed.

Don't repeat yourself / avoid magic numbers

I can see 30 in multiples places. This is usually a bad sign : if you ever want to change the value to something else, you'll have to change it in multiple places. You probably should define a constant to hold that value behind a meaningful name.

Even better, you could define a function to perform the particular behavior that you want :

Getting the length the right way

At the moment, you are keeping track of the number of rows in input_file by incrementing a variable x. It is much clearer to simply use len(intput_file). Also, x = x + 1 can simply be written : x += 1.

Taking these various comments into account, you get :

import csv
SIZE_LINE = 30
def print_with_line(s):
 print(s)
 print('-' * SIZE_LINE)
if __name__ == '__main__':
 # Open template file and pass string to 'data'.
 # Should be in HTML format except with string replace tags.
 with open('testTemplate.htm', 'r') as my_template:
 data = my_template.read()
 # print template for visual cue.
 print_with_line('Template passed:')
 print_with_line(data)
 # open CSV file that contains the data and
 # store to a dictyionary 'input_file'.
 with open('test1.csv') as csv_file:
 input_file = csv.DictReader(csv_file)
 for row in input_file:
 # create filenames for the output HTML files
 filename = 'listing' + row['stockID'] + '.htm'
 # print filenames for visual cue.
 print(filename)
 # create output HTML file.
 with open(filename, 'w') as output_file:
 # run string replace on the template file
 # using items from the data dictionary
 # HELP--> this is where I get nervous because
 # chaos will reign if the tags get mixed up
 # HELP--> is there a way to add identifiers to
 # the tags? like %s1 =row['stockID'], %s2=row['color'] ... ???
 output_file.write(data % (
 row['stockID'],
 row['color'],
 row['material'],
 row['url']))
 # print the number of files created as a cue program has finished.
 print_with_line(str(len(input_file)) + ' files created.')
lang-py

AltStyle によって変換されたページ (->オリジナル) /