2
\$\begingroup\$

In this code in Python, I used an API called CoinDesk to get the Bitcoin price as JSON. I'm trying to follow the best practices in coding, so, I've used logging and I'm using a code formatter in my editor. Kindly, any enhancements on this code will be appreciated.

# Project Idea: Bitcoin price program using CoinDesk API with Python.

# Importing libraries
import requests as rq
from datetime import datetime
import logging
import objectpath  # Read the library doc: http://objectpath.org/


def get_api_data():

    LOG_FORMAT = '%(levelname)s : %(asctime)s - %(message)s'
    logging.basicConfig(level=logging.DEBUG,
                        format=LOG_FORMAT, filemode='w')  # , datefmt='%d-%b-%y %H:%M:%S'
    logger = logging.getLogger()

    try:
        API_LINK = 'https://api.coindesk.com/v1/bpi/currentprice.json'   # CoinDesk API Link
        API_Data = rq.get(API_LINK)
        logger.debug("Getting API data - Done")

        api_data = API_Data.json()
        logger.debug("Converting API data to JSON - Done")
        return api_data

    except Exception as e:
        print("Exception occurred: ", e)
        logger.debug("Exception occurred.")


def main():

    # Getting the data from API
    data = get_api_data()
    tree_data = objectpath.Tree(data)

    time = tuple(tree_data.execute('$..updated'))

    usd = list((tree_data.execute('$..USD')))
    price = dict(usd[0])
    price_usd = price["rate"]
    price_description = price["description"] + ' - USD'

    print(f"The price in {price_description}: {price_usd}")
    print(f"Time of the price: {time}")


if __name__ == '__main__':
    main()
\$\endgroup\$
2
\$\begingroup\$

When using the logger module, you usually want to initialize the logger once per script, at the beginning, and usually with the name of the file (or another descriptive name) as the argument. This way you can distinguish where each logging call comes from (although you have disabled that with your particular choice of formatting string).

Since you are already catching all exceptions, you should probably raise an exception if the connection fails, instead of hoping one is raised when you try to get the JSON object out of the responses.

When you do log exceptions, you are currently printing the exception to the standard output. If you let logging deal with this you could use all functionalities of the module, like outputting to different output streams (stderr usually, for, well, errors). You can either use logger.exception for this or pass the flag exc_info=True. The former uses the logging level logging.ERROR, the latter allows you to use it with any level.

import requests as rq
from datetime import datetime
import logging
import objectpath  # Read the library doc: http://objectpath.org/

logger = logging.getLogger(__name__)

def get_api_data(url):
    try:
        response = rq.get(url)
        response.raise_for_status()
        logger.debug("Getting API data - Done")
        api_data = response.json()
        logger.debug("Converting API data to JSON - Done")
        return api_data
    except Exception as e:
        logger.debug("Exception occurred.", exc_info=True)

def main():
    # CoinDesk API Link
    API_LINK = 'https://api.coindesk.com/v1/bpi/currentprice.json'

    # Getting the data from API
    data = get_api_data(API_LINK)
    tree_data = objectpath.Tree(data)

    time = next(tree_data.execute('$..updated'))
    price = next(tree_data.execute('$..USD'))

    print(f"The price in {price['description']} - USD: {price['rate']}")
    print(f"Time of the price: {time}")


if __name__ == '__main__':
    LOG_FORMAT = '%(levelname)s : %(asctime)s - %(message)s'
    logging.basicConfig(level=logging.DEBUG,
                        format=LOG_FORMAT, filemode='w')
    main()

I also used next instead of tuple and list, which just gets the next element from the generators (no need to get all if you only need one), which also removed the superfluous () around the time, moved the formatting completely into the string, gave the function a parameter to change the URL from the outside and renamed API_DATA to response (since it is not a global constant).

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I'll reconstruct my code according to these valuable notes. Thanks a lot, appreciated. \$\endgroup\$ – Meqdad Dev Mar 16 at 8:00
  • 1
    \$\begingroup\$ I don't think calling basicConfig at the root of the script is good practice, you should at least move it into the if __name__ == '__main__' block. \$\endgroup\$ – SpoonMeiser Mar 16 at 16:59
3
\$\begingroup\$

In general you should be more especific on the management of the exceptions, my only contribution will be to be more specific on them on the requests component. For example your code on:

def get_api_data(url):
    try:
        response = rq.get(url)
        response.raise_for_status()
        ...    
    except Exception as e:
        logger.debug("Exception occurred.", exc_info=True)

With more specific exception management

def get_api_data(url):
    try:
        response = rq.get(url)
        response.raise_for_status()
        ...    
    except requests.exceptions.RequestException as _:
        logger.debug("Exception occurred on request.", exc_info=True)
    except json.decoder.JSONDecodeError as _:
        logger.debug("Exception occurred on json decoder.", exc_info=True)
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks for your contribution. I'll be more specific in the next codes. \$\endgroup\$ – Meqdad Dev Mar 16 at 9:38

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.