HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Improve my script to organize a video collection

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptorganizecollectionimprovevideo

Problem

I've made this script the other day, and I need some input from you telling me how could I improve the code / style. There aren't problems with the code (it does the job) but I want it more organized and "simple" so that in 1 week I still remember what the hell was I thinking!

The problem was: I needed a program that moves for me some files, particularly, some video files, from one directory to another. All the video files are about Tv Shows, so they have this format:

Showname.SxxExx.Episode Title.mp4 (where S = season and E = episode)

But that's not it. I wanted also to create the apropriate folders for every Tv Show i moved in another directory. So if the directory "Show Name" doesn't exists, my script creates it and then checks if the season folder exists inside. If it doesn't, it creates that too and finally it moves the file.

Here is the code: http://bpaste.net/show/148514/

```
import string
import glob
import os
import shutil
import re
import sys
import hashlib
#############################################
from colorama import Fore, Back, init

init(autoreset=True)

#print(Fore.RED + 'some red text')
#print(Back.GREEN + 'and with a green background')
#print(Style.DIM + 'and in dim text')
#print(Fore.RESET + Back.RESET + Style.RESET_ALL)
#print('back to normal now')

#Fore: BLACK, RED, GREEN, YELLOW, BLUE, MAGENTA, CYAN, WHITE, RESET.
#Back: BLACK, RED, GREEN, YELLOW, BLUE, MAGENTA, CYAN, WHITE, RESET.
#Style: DIM, NORMAL, BRIGHT, RESET_ALL

# https://pypi.python.org/pypi/colorama#downloads

#############################################

class TvShow(object):
def __init__(self, path):
self.path = path # The complete path to the video file. Example: D:\Movies\Somefolder\showname.showseason.avi
self.show_name = None # This will hold the show's name. Example: "xFactor" (shit program btw)
self.show_season = None # This will hold the show's season number. Example: "S06" for season 6.
self.show_video_file_name = None # The

Solution

I'm just going to mention a few quick observations rather than do a thorough review.

You're concatenating backslashes everywhere. Use os.path.join() like you're supposed to. (The one place where you call os.path.join(), you're manually adding a backslash unnecessarily.) Conversely, in TvShow.create(), call os.path.split() instead of string.split(…, "\\").

Why does TvShow have an __init__() and a separate create() method? That suggests that your __init__() is a half-assed constructor, producing objects in a not-quite-usable state. If it's possible for objects to be in a weird state, then your class is poorly designed.

Your checkIfValidFolder() does not consistently return a value. Make up your mind what it is supposed to do.

In copyThisShit(), the logic is repetitive. The following flow would work better:

for filename in …:
    if not os.path.exists(to_dir):
        # Dest folder missing? Fix it!
        …
    if not os.path.exists(dest_show_folder):
        # Dest show folder missing? Fix it!
        …
    if not os.path.exists(dest_show_folder_season):
        # File not there? Fix it!
        …


Instead of concatenating blahblah + RED + string + RESET + blah all the time, find a way to call some function to set and reset the color like blahblah + fgRed(string) + blah. It would be cleaner and less error prone.

You have some naming issues. md5_for_file() is your only function_with_underscores. checkInput() is a crazy name for your main function. Who would guess that check…() does more than just checking something? Also, "prog" is usually short for "program", I would have guessed that copy_with_prog() meant "copy using a program" (like copy.exe or rsync), not "copy with a progress bar".

Code Snippets

for filename in …:
    if not os.path.exists(to_dir):
        # Dest folder missing? Fix it!
        …
    if not os.path.exists(dest_show_folder):
        # Dest show folder missing? Fix it!
        …
    if not os.path.exists(dest_show_folder_season):
        # File not there? Fix it!
        …

Context

StackExchange Code Review Q#35137, answer score: 2

Revisions (0)

No revisions yet.