1
\$\begingroup\$

There is a personal program of mine which logs in, scrape links and then download them. The downloader part is based on Youtube-dl and I think it's a smell code. Is there any way to improve this code?

import requests
import subprocess
import getpass
import time
from bs4 import BeautifulSoup as bs
import argparse 


def Login(url, login_route, username, password):

    URL = url
    LOGIN_ROUTE = login_route

    headers = {
        'User-Agent': '',
        'origin': URL,
        'referer': URL + LOGIN_ROUTE,
    }

    request_session = requests.session()

    csrf_token = request_session.get(URL).cookies['csrftoken']

    login_payload = {
        'hidden_username': username,
        'password': password,
        'csrfmiddlewaretoken': csrf_token
    }

    login_request = request_session.post(
        URL + LOGIN_ROUTE, headers=headers, data=login_payload)
    
    # !
    msg = (
        f'You have logged in successfully {login_request.status_code}' if login_request.status_code == 200 else
        f'Error {login_request.status_code}' 
    )

    print(msg)
    

def get_user_input():

    url = 'https://maktabkhooneh.org/signin/?next=/dashboard/'
    username = getpass.getpass('Username: ')
    password = getpass.getpass('Password: ')
    login_route = '/auth/login-authentication'

    return(Login(url, login_route, username, password))


def Scraper(page_url):

    Page_URL = page_url

    headers = {
        'User-Agent': ''
    }

    page = requests.get(
        Page_URL,
        headers=headers,
    )
    soup = bs(page.text, "html.parser")

    URL_List = []
    link_count = 0
    for a_tag in soup.select('a[href^="/course/"]'):

        links = "https://maktabkhooneh.org" + a_tag["href"]

        URL_List.append(links)
        link_count += 1

    return URL_List


def Downloader(url_list):

    URL_List = Scraper(url_list)
    download_count = 0
    for links in URL_List:
        command = f"youtube-dl {links}"
        result = subprocess.call(command, shell=True)
        
        # !
        if result == 0:
             download_count += 1
             return('\nReturned Value', result)

    return(f'\n{download_count} file(s) have been downloaded')



page_url = 'https://maktabkhooneh.org/course/%D8%A2%D9%85%D9%88%D8%B2%D8%B4-%D8%B1%D8%A7%DB%8C%DA%AF%D8%A7%D9%86-%DB%8C%D8%A7%D8%AF%DA%AF%DB%8C%D8%B1%DB%8C-%D9%85%D8%A7%D8%B4%DB%8C%D9%86-Andrew-NG-mk1085/%D9%81%D8%B5%D9%84-%D8%A7%D9%88%D9%84-%D9%85%D9%82%D8%AF%D9%85%D9%87-ch3364/%D9%88%DB%8C%D8%AF%DB%8C%D9%88-%D8%AE%D9%88%D8%B4%D8%A2%D9%85%D8%AF%DB%8C%D8%AF-%DB%8C%D8%A7%D8%AF%DA%AF%DB%8C%D8%B1%DB%8C-%D9%85%D8%A7%D8%B4%DB%8C%D9%86/'

Login_ = input('Login required Website [Y], [N]? ')
# !
if Login_ == 'y' or 'Y':
    get_user_input()

list_len = len(Scraper(page_url))
# !
Download_Permission = input(
    f'\n{list_len} link(s) have been extracted. Do you want to DOWNLOAD them [Y], [N]? ')
# !
if Download_Permission == 'y' or 'Y':
    Downloader(page_url)

Any help would be highly appreciated.

\$\endgroup\$
0
5
\$\begingroup\$

Issues

The first one is the use of the or operator. If you wish to compare a variable with two values, you will have to write the comparison operator twice. For instance,

if Login_ == 'y' or 'Y':
  ...

should be write as:

if Login_ == 'y' or Login_ == 'Y':

Also, it seems you are returning from the Downloader function before iterating though all links (whenever the first process finishes correcly):

...
if result == 0:
  download_count += 1
  # This will return after one commad executes successfully
  return('\nReturned Value', result)

Code style

Now, regarding code smells and style, in python there is PEP-8, a general guide for code style.

For example, functions/methods and variable names should start with lowecase (Login should be called login).

Also, in the Login function, you are assigning the values of the url and login_route parameters to URL and LOGIN_ROUTE but they do not seem to serve for anything special. You could just use the function's parameters directly:

...
headers = {
 'User-Agent': '',
  'origin': url,
  'referer': url + login_route,
}
...
csrf_token = request_session.get(url).cookies['csrftoken']

Same goes for Page_URL in Scraper.

Moreover, return should be used as a statement, rather than with function notation:

def myFunction():
  return 0

Is better than:

def myFunction():
  return(0)

Readability

Lastly, I would say this assignment in Login is not quite readable:

msg = (
        f'You have logged in successfully {login_request.status_code}' if login_request.status_code == 200 else
        f'Error {login_request.status_code}' 
    )

Language features are there for when they are useful, but readability is key and should be kept in mind. I would write that assignment as a simple if-else:

if login_request.status_code == 200:
  msg = f'You have logged in successfully {login_request.status_code}'
else:
  msg = f'Error {login_request.status_code}' 
\$\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.