3
\$\begingroup\$

In this class I made a webscraper for a cryptocurrency website to get the name and the price of each currency. There are maybe API's that I can also use, but I thought it is better to gain some knowledge doing it with scraping.

import requests
from bs4 import BeautifulSoup
import json

class Coins:

    def __init__(self):
        self.url = "https://www.coinbase.com/price"
        self.new_list = []

    # Check if I get a 200 status code and if so, create instance from bs4
    def get_response(self):
        self.response = requests.get(self.url)

        if self.response:
            print("Access granted")
            self.soup = BeautifulSoup(self.response.text, 'lxml')
        else:
            print("Error")

    # Select the class and get the data
    def get_data(self):
        info = self.soup.find_all(class_="Header__StyledHeader-sc-1q6y56a-0 hZxUBM TextElement__Spacer-sc-18l8wi5-0 hpeTzd")
        # Here, I only want to get the name and the price of the crypto, I don't want to get other information and thats why I am using the modulo operator.
        for x,e in enumerate(info):
            if x % 3 == 2:
                # print("I dont need this information")
                pass
            else:
                self.new_list.append(e.text)

        return self.new_list

    # From the information that I got and appended into a list, make a dict
    def make_dict(self):
        self.my_dict = {self.new_list[x]:self.new_list[x + 1] for x in range(0, len(self.new_list), 2)}
        return self.my_dict

    # Save the data into a json file
    def save_data(self):
        with open('data.json','w') as output:
            json.dump(self.my_dict, output, indent=2)




if __name__ == '__main__':
    test = Coins()
    test.get_response()
    test.get_data()
    test.make_dict()
    test.save_data()

One of my concern is: I try to access variables inside a method, is using self a good idea? For example in the method save_data(). I wanted to pass the function get_data() in json.dump() but it didn't work out. So I changed in make_dict() the variable to self.my_dict to access it in the method save_data() and it worked out, but is it a good practice or not at all?

This is my first time actually working with classes. I appreciate any feedback to improve myself.

\$\endgroup\$
2
  • \$\begingroup\$ The use of I tried in the first sentence can indicate that the code is not working as intended. Code review is a site where only working code is reviewed. Is this code working as expected? \$\endgroup\$
    – pacmaninbw
    Commented Oct 19, 2019 at 12:49
  • 1
    \$\begingroup\$ Yes, it is working as expected \$\endgroup\$
    – Number70
    Commented Oct 19, 2019 at 12:54

1 Answer 1

2
\$\begingroup\$

Here is an alternate implementation:

#!/usr/bin/env python3

from bs4 import BeautifulSoup
from pprint import pprint
import re
import requests


class Coins:
    table_re = re.compile('^AssetTable__Table-')

    def __init__(self, url="https://www.coinbase.com/price"):
        self.url = url
        self.session = requests.Session()

    # Check if I get a 200 status code and if so, create instance from bs4
    def _get_response(self) -> BeautifulSoup:
        response = self.session.get(self.url)
        response.raise_for_status()
        return BeautifulSoup(response.text, 'lxml')

    # Select the class and get the data
    def get_data(self) -> dict:
        soup = self._get_response()
        table = soup.find('table', attrs={'class': self.table_re})
        prices = {}
        for row in table.tbody.find_all('tr'):
            cells = row.find_all('td')
            if len(cells) < 6:
                continue
            name = cells[1].find('h4').text
            price = cells[2].find('h4').text
            prices[name] = price
        return prices

Note:

  • Do not rely on long, terrible, possibly dynamically-generated CSS class names
  • If you want to reuse this class to repeatedly fetch prices, then the members should be the session and the URL, and the data should be restricted to the function
  • Don't write access granted. Other than it being corny, you should only throw an exception if there's a failure, not write to stdout if there's a success in the data retrieval method. Such progress indicators are best left to the calling function above.
  • Just index into the cells. Don't use the modulus.
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thanks for your notes. I appreciate the work that you have done for me. I will use your feedback to improve my coding. \$\endgroup\$
    – Number70
    Commented Oct 22, 2019 at 8:15

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.