HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Clean up repeated file.writes, if/elses when adding keys to a dict

Submitted by: @import:stackexchange-codereview··
0
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 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):

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 car


Finally, 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.