# Find and remove duplicate files in one or more directories

I needed this to clean up a couple of similar music libraries, which also contained some empty files and duplicate files with different names.

# dupFinder.py
import os, sys, shutil, datetime
import hashlib

def findDup(parentFolder):
# Dups in format {hash:[names]}
dups = {}
for dirName, subdirs, fileList in os.walk(parentFolder):
print('Scanning %s...' % dirName)
for filename in fileList:
# Get the path to the file
path = os.path.join(dirName, filename)
print(path)
# Calculate hash
file_hash = hashfile(path)
# Add or append the file path
if file_hash in dups:
dups[file_hash].append(path)
else:
dups[file_hash] = [path]
return dups

# Joins two dictionaries
def joinDicts(dict1, dict2):
for key in dict2.keys():
if key in dict1:
dict1[key] = dict1[key] + dict2[key]
else:
dict1[key] = dict2[key]

def hashfile(path, blocksize = 65536):
print(path)
afile = open(path, 'rb')
hasher = hashlib.md5()
while len(buf) > 0:
hasher.update(buf)
afile.close()
return hasher.hexdigest()

def handleResults(dict1):
results = list(filter(lambda x: len(x) > 1, dict1.values()))
if not 'testrun' in globals():
backupdir = 'removed_' + str(datetime.datetime.now())
if not os.path.exists(backupdir):
os.makedirs(backupdir)
else:
backupdir = 'backup_dir_date_now'
if len(results) > 0:
print('Duplicates Found:')
print('The following files are identical. The name could differ, but the content is identical')
print('^^^^^^^^^^^^^^^^^')
count = 0
for result in results:
for subresult in result:
if count == 0:
print('KEEPING ONE COPY: %s.' % subresult)
count += 1
elif 'testrun' in globals():
print('\t\t %s WILL BE REMOVED TO: %s' % (subresult, backupdir))
else:
print('\t\tREMOVING %s TO %s' % (subresult, backupdir))
try:
shutil.move(subresult, backupdir)
except shutil.Error:
print ('\t\t EXISTS. DELETING.')
os.remove(subresult)
print('___________________')

else:
print('No duplicate files found.')

if __name__ == '__main__':
if len(sys.argv) > 1:
dups = {}
if sys.argv[1] == '-t':
testrun = 1
folders = sys.argv[2:]
else: folders = sys.argv[1:]
for i in folders:
# Iterate the folders given
if os.path.exists(i):
# Find the duplicated files and append them to the dups
joinDicts(dups, findDup(i))
else:
print('%s is not a valid path, please verify' % i)
sys.exit()
handleResults(dups)
else:
print('Usage: python dupFinder.py [-t] folder or python [-t] dupFinder.py folder1 folder2 folder3')


It can be run in test mode with -t flag.

Wow - awesome advice here sure transformed the code: GitHub Repo

• Imports should be separated onto multiple lines, not imported on the same line, as seen in the below example.

import import
import os
import sys
import shutil
import datetime


• If you're also going to be using only a couple function/class/variables from a module, you can import them using from ... import ....
• Functions and variables should be in underscore_case, and classes should be in PascalCase.
• You have a lot of code underneath if __name__ == "__main__":. Consider finding a way to separate this code into a main method. if __name__ == "__main__": should usually be about as simple as this.

if __name__ == "__main__":
main( ... )


• You have some inconsistent indentation. E.g, some places are indendented by four spaces/a tab, and others, well, more spaces/tabs.
• Rather than putting comments over functions, use docstrings to describe the purpose of a function, like this.

def my_function(args):
"""
Describe the purpose of your function
here and it's arguments.
"""
...


• My final tip would be to read PEP8, Python's official style guide, and see how you can properly format your code.

If there's anything else that you want me to cover, just mention it in the comments, and I'll see what I can do.

• That's great criticism. I've read PEP8 a couple of times, but obviously could use a review. Why is it that imports are supposed to be on their own line, again? – MikeiLL Jun 14 '15 at 2:19
• @MikeiLL If there's an error associated with a module it makes debugging easier. – Ethan Bierlein Jun 14 '15 at 3:56
• Posting updated version based on above. – MikeiLL Jun 14 '15 at 5:27
• @MikeiLL Do not post your refactored code in your post. It is against site rules. Please see what you may and may not do after receiving answers. – Ethan Bierlein Jun 14 '15 at 5:29
• @MikeiLL The only explanation I can really provide is that it makes your code cleaner and clearer, – Ethan Bierlein Jun 14 '15 at 5:55

If I had to pick a least-favourite line in the program, it would be this one:

if 'testrun' in globals():


… because

• You didn't even declare testrun as a global variable using global testrun, but rather you used introspection.
• You didn't test the flag to see whether it was True or False; you just check for its existence.
• The whole problem could have been avoided by calling handleResults() with a parameter.

print('%s is not a valid path, please verify' % i)
sys.exit()


When you exit the program due to an error, exit with a non-zero status.

i is an unconventional iteration variable, since it has the connotation of being an integer. The code would be more readable if you had written for folder in folders: ….

In hashfile(), you should call open() using a with block.

Since hashfile() seems like it could be a basic reusable operation, I would avoid having it print anything. The findDup() function, which calls hashfile(), prints status updates, and in fact it prints the same path just before calling hashfile().

• Nice. Based on @EthanBierlein's advice had already moved the work into a function to which sys.argv is sent as a parameter. That (i) in the loop bothered me a bit too so thanks for that. Also had wondered about using with in the open block, but thought that maybe with the block/chunksize being declared it wasn't necessary. But the 'controlled_execution' idea makes sense: effbot.org/zone/python-with-statement.htm. Here's current: github.com/MikeiLL/dupFinder in case it's of use to anyone. – MikeiLL Jun 14 '15 at 16:24

Just reviewing the hashfile function.

1. There's no docstring. What does this function do?

2. The MD5 hash is not collision-resistant: this means that an attacker can create two files with the same hash. This could be disastrous if hashfile were used for data integrity or content de-duplication. Use SHA-256 instead.

3. The condition len(buf) > 0 can be written more simply as buf.

4. The duplicated line of code here:

buf = afile.read(blocksize)
while len(buf) > 0:
hasher.update(buf)


can be avoided if you put the loop termination condition in the middle, like this:

while True: