5
\$\begingroup\$

This is a Python script that I use on my Android tablet. It moves the files in the Download folder to a folder on the external SD card. The files are sorted based on their extensions and are moved to folders (for each different type of extension). If it encounters a new file type, it will create a new folder. If the file name already exists, the new file will be renamed by appending the first five hash digits to the end of the file name. The original file is deleted.

How can I improve my program? What changes should I make?

import os
import hashlib

download_dir = "/sdcard/Download/"
save_dir = "/mnt/external_sd/Download/"

FileList = os.listdir(download_dir)
for File in FileList:
    #print File
    extension = ''.join(os.path.splitext(File)[1])
    name = ''.join(os.path.splitext(File)[0])
    ext = extension.strip('.')

    if os.path.isdir(download_dir + File):
        pass
    else:
        if os.path.exists(save_dir + ext + "/" + File):
            Data = open(download_dir + File, "r").read()
            m = hashlib.sha1() 
            m.update(Data) 
            h = (m.hexdigest())[0:5]
            file(save_dir + ext + "/" +name+"-"+h+"."+ext, "w").write(Data)
            print File, " ","-->"," ",name+"-"+h+"."+ext
            os.remove(download_dir + File)

        elif os.path.exists(save_dir + ext):
            Data = open(download_dir + File, "r").read()
            file(save_dir + ext + "/" + File, "w").write(Data)
            print File #, " ","-->"," ",name+"."+ext
            os.remove(download_dir + File)

        if os.path.exists(save_dir + ext) != True:
            os.makedirs(save_dir + ext)
            Data = open(download_dir + File, "r").read()
            file(save_dir + ext + "/" + File, "w").write(Data)
            print File , " ","-->"," ","/"+ext+"/"+name+"."+ext
            os.remove(download_dir + File)
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

File, FileList, and Data are neither class names nor constants; variable names should be lower_case in Python. I'd call it filename instead of file to avoid clashing with Python's built-in file() constructor.

Avoid calling splitext() twice with name, extension = os.path.splitext(filename).

For portability, clarity, and to avoid accidental omission of the path component separator, it would be better to use os.path.join(dirname, filename) instead of dirname + filename, even if you only plan to run it on Android.

You open files for reading and writing but don't close them. In particular, it's important to make sure the destination file handle is successfully closed before removing the source, or you might lose data. Also, it's preferable to call open() instead of file().

The structure of your program can be simplified.

if a:
    ... # body a
else:
    if b:
        ... # body b
    elif c:
        ... # body c
    if c != True:
        ... # body d

can be written as

if a:
    ... # body a
elif b:
    ... # body b
elif c:
    ... # body c
else:
    ... # body d

assuming that bodies b and c do not influence the c != True test. Also, I believe you intend for cases b, c, and d to be mutually exclusive, but I suspect it may be possible to execute both bodies b and d in your original program.

Bodies b, c, and d are very similar to each other. You should try to reduce the redundancy in your code. One approach would be to copy the contents to a tempfile in the destination directory (computing the hash as you copy, in case you need it), then successively try to os.rename() the temporary file to various destination filenames.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.