patternpythonMinor
Improve my script to organize a video collection
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
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
Why does
Your
In
Instead of concatenating
You have some naming issues.
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.