2
\$\begingroup\$

I have written a Scrapy spider to scrape product data from a website. The spider navigates through multiple pages to reach a specific product and extracts details such as the product name, price, color, size, and reviews. I would greatly appreciate it if you could review my code and provide feedback on the following aspects:

  • Correct usage of Scrapy framework features.
  • Adherence to best practices in web scraping.
  • Code efficiency and readability.
  • Error handling and robustness.
  • Overall structure and design.

Here is my code:


import scrapy
import json

class MarksAndSpencerSpider(scrapy.Spider):
    name = "marksandspencer"
    allowed_domains = ["marksandspencer.com"]
    start_urls = ["https://www.marksandspencer.com"]

    def parse(self, response):
        option = response.css('#country-selector-country option[value="BG"]')
        if option:
            value = option.css('::attr(value)').get().lower()
            bg_url = f"https://www.marksandspencer.com/{value}"
            self.logger.info(f"BG URL found: {bg_url}")
            yield response.follow(bg_url, callback=self.parse_bg_page)
        else:
            self.logger.warning("BG option not found on the homepage.")

    def parse_bg_page(self, response):
        men_link = response.css('.nav-item.dropdown.order-lg-3 .subcategory a::attr(href)').get()
        if men_link:
            self.logger.info(f"Men's link found: {men_link}")
            yield response.follow(men_link, callback=self.parse_mens_page)
        else:
            self.logger.warning("Men's section link not found on the BG page.")

    def parse_mens_page(self, response):
        casual_shirts_link = response.xpath('//a[contains(text(), "Casual shirts")]/@href').get()
        if casual_shirts_link:
            self.logger.info(f"Casual shirts link found: {casual_shirts_link}")
            yield response.follow(casual_shirts_link, callback=self.parse_casual_shirts)
        else:
            self.logger.warning("Casual shirts link not found on the Men's page.")

    def parse_casual_shirts(self, response):
        product_link = response.css('div.pdp-link a:contains("Easy Iron Geometric Print Shirt")::attr(href)').get()
        if product_link:
            self.logger.info(f"Product link found: {product_link}")
            yield response.follow(product_link, callback=self.parse_product_page)
        else:
            self.logger.warning("Product link not found in the Casual Shirts section.")

    def parse_product_page(self, response):
        product_name = response.css('.product-name::text').get()
        price = response.css('.value::text').get()
        selected_colour = response.css('button.qa-addtocart-button')
        size = response.css('#plp-select').get()
        reviews_script = response.xpath('(//script[@type="application/ld+json"])[2]/text()').get()

        if reviews_script:
            json_data = json.loads(reviews_script)
            review_count = json_data.get('AggregateRating', {}).get('reviewCount', 0)
            avg_rating = json_data.get('AggregateRating', {}).get('ratingValue', 0.0)
        else:
            review_count = 0
            avg_rating = 0.0

        color = ''
        for button in selected_colour:
            color = button.attrib.get('data-defaultcolor', '')

        sizes = []
        if size:
            select_element = scrapy.Selector(text=size, type="html")
            option_tags = select_element.css('option')
            for option in option_tags:
                option_text = option.xpath('normalize-space(text())').get()
                if option_text != "Select Size":
                    sizes.append(option_text)

        product_data = {
            "name": product_name or '',
            "price": price or '',
            "colour": color,
            "size": sizes,
            "reviews_count": int(review_count),
            "reviews_score": float(avg_rating),
        }

        yield product_data



I am eager to learn and improve, so any suggestions or advice you can provide will be very helpful.

\$\endgroup\$
1
  • 2
    \$\begingroup\$ Welcome to Code Review! I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to edit and give it a different title if there is something more appropriate. \$\endgroup\$ Commented Jun 27 at 20:27

1 Answer 1

2
\$\begingroup\$

Overall, it looks great.

deprecated class

Doing $ scrapy runspider *.py reveals that the OP code chooses to use the 2.6 REQUEST_FINGERPRINTER_IMPLEMENTATION. It has been deprecated since 2022. The documentation explains that

[The 2.7] implementation was introduced in Scrapy 2.7 to fix an issue of the previous implementation.

New projects should use this value.

Just add this near the top, after start_urls:

    custom_settings = {
        "REQUEST_FINGERPRINTER_IMPLEMENTATION": "2.7"
    }

silence warnings

In general, with any computer technology, pay attention to diagnostic warnings that are telling you to do something different. Decide whether you agree with the advice, and then either

  • heed the advice, so there's no warning, or
  • silence the warning message, so you needn't consider it again.

hardcode

Recall that each spider has a list of seed URLs to crawl, though the OP code currently cares about just a single site.

            bg_url = f"https://www.marksandspencer.com/{value}"

That seems more brittle than desired. It would be much better to pull it from response.url, which we see points to exactly the desired place. Subsequent code does a fine job of building up next URL based on current URL.

redirect

Also, I can't imagine why you prefer to specify a path of /bg rather than /bg/. The former incurs the cost of a 301 redirect.

extract helper

Your CSS selectors look terrific, very sensible.

The parse_bg_page(), parse_mens_page(), and parse_casual_shirts() methods all seem nice enough, but they're tediously repetitive. It seems like a dict which maps from response.url to selector could drive some common helper function. That would eliminate the copy-pasta we see here, DRYing up the code.

Actually, we need to map an URL to a (selector, parser_function) tuple. Most entries would point back to that same helper function, and the final entry would mention parse_product_page. Note that the tuple needs a function reference, so we do not write parse_product_page(), as the parentheses would call the function rather than produce a reference to it. Your various callback= arguments do a good job of illustrating this.

extra whitespace

        product_data = {
            ...
            "price": price or "",

I like the "or empty-string" idiom, very compact.

This is storing "\nBGN 115.00\n", which seems an inconvenient value. Prefer price.strip() to fix it.

additional data cleaning

I see both "amount in stock" and "size" conflated in a single field:

  "size": [
    "S-Regular Out of Stock",
    "M-Regular",
    "L-Regular",
    "XL-Regular",
    "2XL-Regular Out of Stock",
    "3XL-Regular Low in Stock",
    "4XL-Regular Out of Stock",
    "5XL-Regular Out of Stock"
  ],

So you might have some additional data cleaning code to write for that.


This codebase achieves its design goals.

I would be willing to delegate or accept maintenance tasks on it.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you so much for valuable answer! I will modify it according to your recommendations and will post it again for final review. Kind Regards \$\endgroup\$ Commented Jun 28 at 4:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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