I wrote this script to download images from Reddit. I would like to hear from others on how can I improve this script.

import requests as _requests
import os as _os

class Redpy:

    def __init__(self, user):
        """Enter a user agent"""
        self.user = user

    def download(self, subreddit, number=5, sort_option=None):
        """Downloads images from subreddit.
            subreddit="Name of subreddit"
            number=Number of images to be downloaded
        if sort_option == None:
            sort_option = ''

        self.url = 'https://www.reddit.com/r/' + subreddit + '/'+  sort_option  +  '.json'

        self.user = {'user-agent':self.user}
        res = _requests.get(self.url, headers=self.user)

        if res.status_code != 200:
            print("Could not download")

        self._DownloadFiles(res.json(), number)

    def _DownloadFiles(self, jsonfile, number_of_files):
        image_links = self._getImages(jsonfile, number_of_files)

        if not self.createFolder():
            print("Error creating folder")

        index = 0 #used to name the files
        for image_link in image_links:
            image_link = image_link.replace('amp;', '')
            f = _requests.get(image_link)

            if f.status_code==200:
                media_file = open(f'{_os.getcwd()}/red_media/{index}.jpg', 'wb')

                for chunk in f.iter_content(100000):
        print("Download complete")
        global flag

    def _getImages(self, jsonfile, number_of_files):

        images = [] #contains links of images
        for index in range(number_of_files):
            except Exception as e:
        return images

    def createFolder():
            if not _os.path.exists(f'{_os.getcwd()}\\red_media'):
                return True
            return True
        except Exception as e:
            return False

Things I would like to know:

  • Is there anything that I can do to improve the performance of the code
  • What can I do to improve the code styling. I have tried to follow the PEP standards as much as I could.
  • Anything else to improve the code.

A few suggestions for general code quality:

  • If your class only does one thing and it doesn't store any values, it should be a function. Redpy only downloads images from reddit and stores values to achieve exactly this, which you could do in a function. Using a class can have unforeseen consequences.
  • Choose descriptive names for variables and functions. _getImages does not actually get the images, it returns a list of links of images. In this method, you have images = [] #contains links of images. The comment could have been avoided if you would have chosen image_links as name.
  • If you split your code up into methods or functions, everything belonging to one task should be inside it. The removal of 'amp;' in every image_link does not belong in _DownloadFiles, it should be in _getImages. download gets unnecessarily separated into _DownloadFiles and _DownloadFiles doesn't generally download files, but it could if some of its functionality got relocated elsewhere.
  • Clean up your code: there are unnecessary line breaks after _DownloadFiles and a redundant return True in createFolder.
  • Don't catch general Exceptions, be more specific. In _getImages, you should just look out for KeyErrors. Exceptions in request.get on the other hand are not handled although they possibly should be.
  • The pattern of looping over a list with a counter (index in your code) in _DownloadFiles can be simplified with enumerate.
  • When working with files, it is more elegant to use a context manager.

Possible bugs:

  • subreddit.strip('/') just returns a new string that you would have to assign to a new variable. In your code, the value of subreddit remains unchanged.
  • self.user gets updated every time download is called. If this happens multiple times, self.user becomes a dict encapsulating a dict encapsulated a dict...
  • If something goes wrong when extracting links in _getImages, less links than expected get returned.
  • If your folder already contains images, they will be overwritten.

Concerning PEP8:

  • A few of your lines are longer than 80 characters. Try to split them up, either by implementing the same logic over multiple lines or by breaking the line up.
  • In PEP8, functions and methods are in snake_case.

Nit-picky stuff:

  • You could just use an empty string as default argument for sort_option. Strings are immutable, so you don't have the problem of mutable default arguments.
  • I don't see why you would import requests as _requests and os as _os
  • There is no need to construct an absolute file path. f'{_os.getcwd()}/red_media/{index}.jpg' could become f'red_media/{index}.jpg'

Here is my attempt at solving this problem:

import requests
import os

def get_image_links(json, N):
    '''returs a list of the first <N> links to reddit images found in <json>'''
        children = json['data']['children']
    except KeyError:
        return []

    # append links from children until N are found
    image_links = []
    for child in children:
            image_link = child['data']['preview']['images'][0]['source']['url']
        except KeyError:

        image_link = image_link.replace('amp;', '')

        if len(image_links)==N:

    return image_links

def download_files(file_links, folder_name='data', file_extension='jpeg'):
    '''downloads files from <file_links> into subfolder ./<folder_name>/'''

    # create subfolder if it does not exist
    if not os.path.exists(folder_name):

    # download files
    for i, file_link in enumerate(file_links):
            res = requests.get(file_link)
        except requests.exceptions.RequestException:
            print(f"Unable to download {file_link}")
        if not res.ok:
            print(f"Error {res.status_code} when requesting {file_link}")

        file_path = os.path.join(folder_name, f'{i}.{file_extension}')
        with open(file_path, 'wb') as file:
            for chunk in res.iter_content(100000):

def download_reddit_images(user, subreddit, N=5, sort_by='',
    downloads the first <N> images of <subreddit> sorted by <sort_by>
    (''/'new'/'hot'/'top') into subfolder ./<folder_name>/

    json_url = ('https://www.reddit.com/r/' + subreddit.strip('/') + '/'
                + sort_by + '.json')

        res = requests.get(json_url, headers={'user-agent':user})
    except requests.exceptions.RequestException:
        print(f"Unable to get {json_url}")

    if not res.ok:
        print(f"Error {res.status_code} when requesting {json_url}")

    image_links = get_image_links(res.json(), N)
    if not len(image_links):
        print("Unable to find any images")

    download_files(image_links, folder_name=folder_name, file_extension='jpeg')

if __name__=='__main__':
    download_reddit_images('', 'all')

The problem of overwriting existing images persists. A solution would be to use the original filename from reddit that is included in the url.

  • 1
    \$\begingroup\$ Thanks a lot for your answer. Your answer was the most comprehensive and detailed analysis of the code. I had updated the code after following some PEP 8 guidelines. Here it is github.com/prashantsengar/RedPy ... I will update the code again based on your suggestions. I have a question though, how can using a class have unforeseen consequences. I want to know more about it. And thanks again for your valuable input \$\endgroup\$ – Prashant Sengar May 22 '19 at 12:31
  • 1
    \$\begingroup\$ By unforeseen consequences I meant bugs like the one with self.user, since you wouldn't expect the code execution to be dependant on previous calls. The problem with using a class here is that it makes the code more complex than it has to be, while a function would work just fine. With the class, it also takes more lines to access the functionality of your code, since you have to create a Redpy object and then call download, whereas the only thing you gain is not having to enter user multiple times. For more information see this talk. \$\endgroup\$ – reeetooo May 22 '19 at 16:28
  • 1
    \$\begingroup\$ Thanks again for explaining so nicely @reeetooo \$\endgroup\$ – Prashant Sengar May 22 '19 at 17:38
  • 1
    \$\begingroup\$ I would probably make a download_file function that just downloads a file to a destination and do the iterating over the links directly in download_reddit_images. But otherwise a very nice first review, welcome to Code Review! \$\endgroup\$ – Graipher May 25 '19 at 10:03

Your Answer

