1

I am a new python programmer and have been making a file sort function to take a file name and neatly arrange it in a file structure being year/month/day. The following code works but looks ugly and there is a lot of duplicate exception errors which I would like to remove. Would love to see how to improve the efficiency of this code as it will be run frequently. Thanks in advance

def fileSort(day, month, year, file):
    global filewritten

    try: os.makedirs(togoto + '/' + year)
    except FileExistsError:
        pass
    try: os.makedirs(togoto + '/' + year + '/' + month)
    except FileExistsError:
        pass
    try:
        os.makedirs(togoto + '/' + year + '/' + month + '/' + day)
    except FileExistsError:
    pass
    try:
        shutil.move(path + '/' + file,
                    togoto + '/' + year + '/' + month + '/' + day + '/' + file)
        filewritten += 1

    except FileExistsError:
        pass
rup
  • 483
  • 5
  • 15
  • You could make a decision block seeing if each directory exists using `os.path.isdir` starting with the longest path for the first check and moving backwards. If the directory already exists, it would decrease your work. – I Funball Mar 06 '19 at 16:11
  • this answer might help https://stackoverflow.com/a/14364249/2484882 – ekauffmann Mar 06 '19 at 16:12

3 Answers3

1

You could define your own function to shorten the code and it's also good for re-usability:

def create_dir(name):
    try:
        os.makedirs(name)
    except FileExistsError:
        pass

def fileSort(day, month, year, file):
    global filewritten
    create_dir(togoto + '/' + year)
    create_dir(togoto + '/' + year + '/' + month)
    create_dir(togoto + '/' + year + '/' + month + '/' + day)
    try:
        shutil.move(path + '/' + file, togoto + '/' + year + '/' + month + '/' + day + '/' + file)
        filewritten += 1
    except FileExistsError:
        pass
DjaouadNM
  • 22,013
  • 4
  • 33
  • 55
1

os.makedirs() already creates the directories leading to the given path, so it should be enough to do

try:
    os.makedirs(togoto + '/' + year + '/' + month + '/' + day)
except FileExistsError:
    pass
try:
    shutil.move(path + '/' + file,
                togoto + '/' + year + '/' + month + '/' + day + '/' + file)
    filewritten += 1

except FileExistsError:
    pass

This is a bit of an improvement to your original version.

BTW, os.path.join() is your friend:

source = os.path.join(path, file)
targetdir = os.path.join(togoto, year, month, day)
target = os.path.join(togoto, year, month, day, file)
try:
    os.makedirs(targetdir)
except FileExistsError:
    pass
try:
    shutil.move(source, target)
    filewritten += 1

except FileExistsError:
    pass

Even better would be to use all the capabilities of os.makedirs() if your Python is new enough:

source = os.path.join(path, file)
targetdir = os.path.join(togoto, year, month, day)
target = os.path.join(targetdir, file)

os.makedirs(targetdir, exist_ok=True) # i. e. no exception on an already existing path.
try:
    shutil.move(source, target)
    filewritten += 1
except FileExistsError:
    pass
glglgl
  • 89,107
  • 13
  • 149
  • 217
1

First of all: use makedirs with only the innermost directory:

try: 
    os.makedirs(togoto + '/' + year + '/' + month + '/' + day)
except FileExistsError:
    pass

Then notice that you should probably use os.path.join to form the path instead, hence:

try: 
    os.makedirs(os.path.join(togoto, year, month, day))
except FileExistsError:
    pass 

And... in Python 3 (3.2+) there is a parameter exists_ok that can be set to True so that no exception is thrown if the leaf child directory exists, so we get

os.makedirs(os.path.join(togoto, year, month, day), exists_ok=True)

Finally do note that shutil.move might - or might not - throw a FileExistsError if the target exists...