Problem
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)
Solution
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.