4
\$\begingroup\$

I'm fairly good at programming python but of course i want to write cleaner code.

This is a simple script that scrapes leads off yellowbook.

I want to utilize list comprehensions but im comfortable with using for loops, however i can see how it can be repetitive.

Any ways i can make this cleaner ?

service_name = input("Input Industry: ")
city = input("Input The City: ")

class Item(scrapy.Item):
    business_name = scrapy.Field()
    phone_number = scrapy.Field()
    website = scrapy.Field()


class QuotesSpider(scrapy.Spider):
    name = "quotes"

    start_urls = [
        "http://www.yellowbook.com/s/" + service_name + "/" + city 
    ]
    def __init__(self):
        self.seen_business_names = []
        self.seen_websites = []


    def parse(self, response):
        for business in response.css('div.listing-info'):
            item = Item()
            item['business_name'] = business.css('div.info.l h2 a::text').extract()
            item['website'] = business.css('a.s_website::attr(href)').extract()
            for x in item['business_name'] and item['website']:
                #new code here, call to self.seen_business_names
                if (x not in self.seen_business_names and x not in self.seen_websites):
                    if item['business_name']:
                        if item['website']:
                            item['phone_number'] = business.css('div.phone-number::text').extract_first()
                            yield item
                            self.seen_business_names.append(x)




        # next_page = response.css('div.pagination a::attr(href)').extract()
        for href in response.css('ul.page-nav.r li a::attr(href)'):
            yield response.follow(href, self.parse)
\$\endgroup\$
1
  • \$\begingroup\$ for x in item['business_name'] and item['website']: is this really doing what you expect it to? I mean, maybe you really wanted to write if bool(item['business_name']) and bool(item['website']): for x in item['website']: in a shorter way, but the rest of the code looks more like you wanted to write for x in item['business_name'] + item['website']: \$\endgroup\$ – 301_Moved_Permanently Mar 9 '18 at 15:06
4
\$\begingroup\$

First: a general observation - simple nested if statements are equivalent to a single if with statements joined by and.

So:

if a:
    if b:
        if c:

Is equivalent to:

if a and b and c:

Second: You have an if test to see if business_name and website exist - but you do a lot of other things before this test. You can move this higher up so you 'escape' that code faster if you do something like:

item['business_name'] = business.css('div.info.l h2 a::text').extract()
item['website'] = business.css('a.s_website::attr(href)').extract()
if item['business_name'] and item['website']:
    for x in item['business_name'] and item['website']:
        #new code here, call to self.seen_business_names
        if (x not in self.seen_business_names and x not in self.seen_websites):
            item['phone_number'] = business.css('div.phone-number::text').extract_first()
            self.seen_business_names.append(x)
            yield item

(I've put the append before the yield as this seems slightly clearer to me).

Third - I don't think you mean for x in item['business_name'] and item['website']:

Rather can do either of:

for x in item['business_name'] + item['website']:

Or:

from itertools import chain
for x in chain(item['business_name'], item['website']):
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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