This code uses the YouTube API for crawling through a YouTube channel, parsing responses and syncing the channel videos with a local directory.

Can this be optimised or improved?

import subprocess
import os
import urllib.request as urlreq
import urllib.parse as urlparse
import simplejson as json

playlistIds = []
titles = []

def getPlaylistId():
    data = {}
    data['maxResults'] = '50'
    data['channelId'] = 'UCtESv1e7ntJaLJYKIO1FoYw' # Put the channelId of channel you want to Sync to.
    data['part'] = 'snippet'
    data['key'] =   'AIzaSyAngcF6oKnyEbhk3KyL9Wz1OhSi28JjbzE'
    requestValues = urlparse.urlencode(data)
    request = "https://www.googleapis.com/youtube/v3/playlists?" + requestValues
    string = urlreq.urlopen(request).read().decode('utf-8')
    items = json.loads(string)['items']

    for item in items:
        playlistIds.append(item['id'])
        titles.append(item['snippet']['title'])

def download():
    for ids,title in zip(playlistIds,titles):
            url = "https://www.youtube.com/playlist?list=" + ids
            if not os.path.exists(title):
                os.makedirs(title)
            os.chdir("./" + title)
            url_down = "youtube-dl -o '%(title)s' " + url
            subprocess.call(url_down, shell=True)
            os.chdir("..")

getPlaylistId()
download()
up vote 1 down vote accepted

Global variables are almost never needed. You should pass playlistIds as a parameter. Well, actually, just don't make getPlaylistId() modify the list; make it return the list. Even better, make it a generator function that yields each id and title. That's better because you aren't storing the id's and titles in memory.

Your naming is not in accordance with PEP 8, the Python style guide:

Function names should be lowercase, with words separated by underscores as necessary to improve readability.

mixedCase is allowed only in contexts where that's already the prevailing style (e.g. threading.py), to retain backwards compatibility.

and for Method Names and Instance Variables:

Use the function naming rules: lowercase with words separated by underscores as necessary to improve readability.

You seem to have some magic values such as '50', 'UCtESv1e7ntJaLJYKIO1FoYw', etc. How did you come up with those values? I would suggest making constants at the beginning of the file for your defaults. Then, use sys.argv or, if you want to be fancier, argparse to get the values from the user.

Instead of creating a dictionary (data) and then setting all of the values, why not just create a dictionary with the right values in the first place? That is, data = {'maxResults': '50', 'channelId': 'UctESv1e7ntJaLJYKIO1FoYw', ...}

Instead of defining items just to iterate, why not for item in json.loads(string)['items']:?

Your indentation in download() is odd. You use four spaces everywhere else, but here you use eight. Be consistent at the least.

os.chdir("./" + title)

Don't make it a habit to put the dots and the slashes in yourself. Use os.path.join(os.curdir, title). os.chdir("..") also would change to os.chdir(os.pardir).

Let's say I want to use your program in mine, but I have a different list of id's to get and a different list of titles. This is my program:

from downloader import download

def get_ids_and_titles():
    ...
download(get_ids_and_titles())

After the first line, your program just did its thing. It downloaded everything. I didn't want to run your program; I wanted to use one of its functions. You can find out if your program is being run as a module or as a program like this:

if __name__ == '__main__':
    download(getPlaylistId())

If your program is run as a program, __name__ is __main__, but if it is a module, it will be the name of the module.

Your Answer

 
discard

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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