# Little command line video grabber

I made a little Python3 script to grab video clips from a website for viewing off-line later and would like your critique, advice, tips & tricks and/or learning resources to create better code. (Project on GitHub)

#!/usr/bin/env python3

import os
import sys
import argparse
import urllib.request
import bs4

def reporthook(blocknum, blocksize, totalsize):
''' Define the reporthook for url-retrieval
(displays info while grabbing). '''
if totalsize > 0:
percent = readsofar * 1e2 / totalsize
display = "\r%*d / %d %5.0f%%" % (
sys.stdout.write(display)
sys.stdout.write("\n")
else:   # total size unknown

PARSER = argparse.ArgumentParser(
help="a ten-digit clipcode.")
help="A http proxy to use; e.g. http://localhost:8000")

PROXY = PARSER.parse_args().proxy
CLIPCODES = PARSER.parse_args().clipcode

if PROXY:
print("Grabbing through http proxy: %s\n" % PROXY)
os.environ['http_proxy'] = PROXY

for i in CLIPCODES:
if not i.isdigit() or not len(i) == 10:
sys.exit(
"'\033[1m%s\033[0m' is not a valid ten-digit clipcode," % i,

for i in CLIPCODES:
soup = bs4.BeautifulSoup(
urllib.request.urlopen("http://somesite.com/%s" % i))
title = soup.find("h1", class_="video-title").string
print("Grabbing \033[1m%s\033[0m:" % i)
href, '%s - %s.mp4' % (i, title), reporthook)
print("\033[1m", local_filename, "Done\033[0m")

• I felt that the actual used site was not very relevant to your code question, so I decided to edit your post and remove the actual site. This code will probably work for more than one site. – Simon Forsberg Jan 30 '15 at 15:25
• Hi @simon-andré-forsberg, I wrote this script for this site and did not (yet) consider usability for other sites and, at first glance, have not found other use-cases. I'll check it out, Thanks! – markhamden Jan 30 '15 at 21:44
• Either way, you should be careful with what links you spread. – Simon Forsberg Jan 30 '15 at 21:49

### A bug

This just won't work:

sys.exit(
"'\033[1m%s\033[0m' is not a valid ten-digit clipcode," % i,


sys.exit takes a single integer parameter to use as the status code of the program, so that callers can check if the program terminated normally or failed. I suppose you wanted to print instead and then sys.exit(1).

### Move all code to methods

Don't keep code in the global namespace. A good practice is to move everything to methods, and call them from inside a if __name__ == "__main__": guard, like this:

def parse_args():
# arg parser code

def main():
args = parse_args()
proxy = args.proxy
clipcodes = args.clipcode

if __name__ == "__main__":
main()


This way it will be possible to import the module into a larger app later. Another advantage is that it doesn't pollute the global namespace, which could lead to obscure bugs.

### Use print() if you want to print

Printing with sys.stdout.write is very unusual and tedious. Using print() is more common and simple. If you don't want to print a newline, use the end='' flag, for example:

print(message, end='')


### Naming

i is really a very poor name in this loop:

for i in CLIPCODES:
if not i.isdigit() or not len(i) == 10:
# ...


It would be better to use something more descriptive, for example clipcode.

Another thing here, not len(i) == 10 is a very odd way of writing len(i) != 10.

It's very unusual to see ALL_CAPS for variable names that are not global constants. For example DEFAULT_PROXY_PORT = 8080 would make a fine global constant, but these look really odd:

PARSER = argparse.ArgumentParser(...)
CLIPCODES = PARSER.parse_args().clipcode

• Hi @janos, many thanks for your help & advice! I'll implement & update (and read some more about the aforementioned topics) ASAP. – markhamden Jan 30 '15 at 21:54