patternpythonMinor
Clean up repeated file.writes, if/elses when adding keys to a dict
Viewed 0 times
cleandictfilewritesrepeatedaddingkeyswhenelses
Problem
I'm getting familiar with python and I'm still learning it's tricks and idioms.
Is there an better way to implement
In
Is there anything that is flat out wrong and should be avoided/improved?
```
#!/usr/bin/python
import requests
import bs4
import os
### Declarations
home = os.environ['HOME']
filename = home + '/results.html'
results_file = open(filename, 'w')
req = requests.get([url_removed])
soup = bs4.BeautifulSoup(req.content)
listings = soup.find_all('div', class_='listing listing-findcar listing-dealer ')
listings.reverse()
### Functions
def print_header(message, html_file):
html_file.write(message)
def print_html(car, html_file):
price = '' + car['price'].strip() + ''
image = ''
year = '' + car['year'].strip() + ' ' + car['trim'].strip() + ''
color = '' + car['color'].strip() + ''
miles = '' + car['miles'].strip() + ''
dealer = '' + car['dealer'].strip() + ''
phone = '' + car['phone'].strip() + ''
distance = '' + car['distance'].strip() + ''
link = '[text_removed]'
html_file.write(price)
html_file.write(image)
html_file.write('')
html_file.write(year)
html_file.write(color)
html_file.write(miles)
html_file.write(dealer)
html_file.write(phone)
html_file.write(distance)
html_file.write(link)
html_file.write('')
def get_car(listing):
car = dict()
# Year
if listing.find('span', class_='atcui-truncate ymm'):
car['year'] = listing.find('span', class_='atcui-truncate ymm').get_text()
else:
car['year'] = ''
# Trim
if listing.find('span', class_='trim'):
car['trim'] = listing.find('span', class_='trim').ge
Is there an better way to implement
print_html() without the multiple calls to html_file.write()? I only separated the HTML for clarity instead of putting the entire string inside write().In
get_car(), is there a way to avoid using if/else to make the dict key ' ' if listing.find() returns nothing? Is there anything that is flat out wrong and should be avoided/improved?
```
#!/usr/bin/python
import requests
import bs4
import os
### Declarations
home = os.environ['HOME']
filename = home + '/results.html'
results_file = open(filename, 'w')
req = requests.get([url_removed])
soup = bs4.BeautifulSoup(req.content)
listings = soup.find_all('div', class_='listing listing-findcar listing-dealer ')
listings.reverse()
### Functions
def print_header(message, html_file):
html_file.write(message)
def print_html(car, html_file):
price = '' + car['price'].strip() + ''
image = ''
year = '' + car['year'].strip() + ' ' + car['trim'].strip() + ''
color = '' + car['color'].strip() + ''
miles = '' + car['miles'].strip() + ''
dealer = '' + car['dealer'].strip() + ''
phone = '' + car['phone'].strip() + ''
distance = '' + car['distance'].strip() + ''
link = '[text_removed]'
html_file.write(price)
html_file.write(image)
html_file.write('')
html_file.write(year)
html_file.write(color)
html_file.write(miles)
html_file.write(dealer)
html_file.write(phone)
html_file.write(distance)
html_file.write(link)
html_file.write('')
def get_car(listing):
car = dict()
# Year
if listing.find('span', class_='atcui-truncate ymm'):
car['year'] = listing.find('span', class_='atcui-truncate ymm').get_text()
else:
car['year'] = ''
# Trim
if listing.find('span', class_='trim'):
car['trim'] = listing.find('span', class_='trim').ge
Solution
You can make the HTML a multi-line string with formatting templates (see this SO question for discussion on indentation, I've kept it simple):
Then your writing becomes a single line:
Note that this doesn't
There is a lot of duplication in
Finally, you aren't always following the style guide, e.g. lining up operators:
def print_html(car, html_file):
html = '''{0[price]}
{0[year]} {0[trim]}
...
'''Then your writing becomes a single line:
html_file.write(html.format(car))Note that this doesn't
strip; you should do that when the data go into the dictionary. There is a lot of duplication in
get_car. Also, you do the find twice, which will slow you down. Try something like:def get_car(listing):
car = dict()
data = {'year': ('span', 'atcui-truncate ymm'),
...}
for key in data:
element, class_ = data[key]
result = listing.find(element, class_=class_)
car[key] = '' if result is None else result.get_text().strip()
return carFinally, you aren't always following the style guide, e.g. lining up operators:
### Declarations
home = os.environ['HOME']
filename = home + '/results.html'
results_file = open(filename, 'w')Code Snippets
def print_html(car, html_file):
html = '''<h3>{0[price]}</h3>
<img src="{0[image]}">
<ul>
<li>{0[year]} {0[trim]}</li>
...
</ul>'''html_file.write(html.format(car))def get_car(listing):
car = dict()
data = {'year': ('span', 'atcui-truncate ymm'),
...}
for key in data:
element, class_ = data[key]
result = listing.find(element, class_=class_)
car[key] = '' if result is None else result.get_text().strip()
return car### Declarations
home = os.environ['HOME']
filename = home + '/results.html'
results_file = open(filename, 'w')Context
StackExchange Code Review Q#55522, answer score: 5
Revisions (0)
No revisions yet.