6
\$\begingroup\$

I'm working on a project for digital marketing, I want to automate the upload of pictures. The program itself isn't done but here I came up with a way to keep track of which and when pictures have been posted.

from os import listdir, rename
from random import choice
from _datetime import datetime

# The main directory in which I'll store the pictures
pictures_dir = "C:/Users/firmi/Desktop/Pictures/"

# The directory in which posted pictures go to
pictures_posted_dir = pictures_dir + "Posted/"

# The folders inside the main directory
picture_folders = listdir(pictures_dir)

# List of folders with pictures that have not been posted
pictures_to_post = [kind for kind in picture_folders if not kind.startswith('Posted') and len(listdir(pictures_dir + kind)) > 0]

# Current date of the post
post_time = datetime.now().strftime("_(%d-%m-%Y)")

def post_picture(picture_dir, kind):
    # Here I move the picture to the 'posted' folder
    rename(picture_dir, pictures_posted_dir + kind + str(post_time) + ".png")

def randomize_picture():
    # Here a random picture is selected(if there are any)
    if len(pictures_to_post) == 0:
        print('No Pictures Left')
    else:
        random_kind = choice(pictures_to_post)
        random_kind_dir = pictures_dir + random_kind + "/"
        random_picture = choice(listdir(random_kind_dir))
        random_picture_dir = random_kind_dir + random_picture

        post_picture(random_picture_dir, random_kind)

randomize_picture()

It's all very generic because I want to be able to have multiple 'main folders' and they are all going to have their kinds of pictures. Is there any way I can improve this code?

\$\endgroup\$
3
\$\begingroup\$

I'd recommend having a look at Python's "official" Style Guide aka PEP 8, which is widely accepted in the community as a guideline on how Python code should be formatted.

But you can also learn a lot from looking at other peoples code, e.g. here on Code Review. E.g. it's very common to have imports at the top of the file, followed by constants, class and function definitions, with "script" code most often found at the very end. The script code is often surrounded by if __name__ == "__main__":, which is a way to tell the interpreter to only run this piece of code in case the file is executed as a script (and not when you try to import a function from it).

Directly applying this to your code might leave you with an awkward feeling, because some global variables are used inside the functions before they are "physically" defined in the script. You've probably heard this from other people already: try to avoid global variables whenever possible. This is one of the reasons. In general, global variables make it harder so see what goes into a function and how the function affects the "state"/variables of your program without looking at the source code. If all inputs are passed as parameters and the new/altered values are actually returned from the function, you don't need to know what exactly happens inside to understand what's going on.

You work around this be passing all the necessary variables as parameters when calling a function.

If that brings up another strange feeling, like say, a function accepts parameters that seem to have nothing to do with what it name would let one expect, then you have identified another common error: violations of the single responsibility principle. A few ideas how that might look like in your script

  1. Have a main() function where all the script code lives. This function calls other functions as necessary and provides input to them via parameters.

  2. Have randomize_picture accept all the necessary inputs as parameter and return which image was selected, raise an appropriate exception whenever this is not possible instead of printing to console.

  3. Pass the source image and all the other relevant information to post_image(...).

  4. Alternatively, assemble the target filename also in main so you'll just have to pass two parameters to post_image(...).

import os
from os import listdir, rename
from random import choice
from datetime import datetime


def post_picture(original_picture, pictures_posted_dir, kind, post_time):
    """Here I move the picture to the 'posted' folder"""
    rename(original_picture, pictures_posted_dir + kind + post_time + ".png")


def randomize_picture(pictures_to_post, pictures_dir):
    """Here a random picture is selected(if there are any)"""
    if not pictures_to_post:
        raise ValueError('No Pictures Left')

    random_kind = choice(pictures_to_post)
    random_kind_dir = pictures_dir + random_kind + "/"
    random_picture = choice(listdir(random_kind_dir))
    random_picture_dir = random_kind_dir + random_picture

    return random_picture_dir, random_kind


def main():
    # The main directory in which I'll store the pictures
    pictures_dir = "C:/Users/firmi/Desktop/Pictures/"

    # The directory in which posted pictures go to
    pictures_posted_dir = os.path.join(pictures_dir, "Posted/")

    # The folders inside the main directory
    picture_folders = listdir(pictures_dir)

    # List of folders with pictures that have not been posted
    pictures_to_post = [kind for kind in picture_folders if not kind.startswith('Posted') and len(listdir(pictures_dir + kind)) > 0]

    # Current date of the post
    post_time = datetime.now().strftime("_(%d-%m-%Y)")

    random_picture_dir, random_kind = randomize_picture(pictures_to_post, pictures_dir)

    post_picture(random_picture_dir, pictures_posted_dir, random_kind, post_time)

A few notes on what else has changed:

  • import _datetime --> impor datetime. Variables, functions and modules starting with a leading _ are not supposed to be used in code outside of that file or module. It's a Python convention to tell others, that you consider it an implementation detail which is subject to change. So don't use it.
  • the function documentation was changed from # comments to proper docstrings. docstrings are found by Python's built-in help(...) function as well as by most IDEs and are THE way to go for function, class, and module documentation.
  • whenever working with paths it's best to use either os.path.join (Python 2/3) or the pathlib module. Both of them make working with paths much more convenient, e.g. because they know which separator to use for which platform (Linux: /, Windows: \).
  • Look Ma! No global variables ;-)
|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Yeah man, many people have already told me that i should not have global variables, but they didn´t explain it very much. Thank you for your suggestions, I´ll take a look at the links you gave :) \$\endgroup\$ – Tlomoloko Jan 21 at 0:26
  • 1
    \$\begingroup\$ I added a bit more on that to the answer (previously here in a comment). \$\endgroup\$ – AlexV Jan 21 at 9:28

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.