Removing duplicate file references from history

Question

I have some code that searches for duplicate files and then removes those files from a csv. To make the set of files I am searching over much smaller, I first group the files by size; if a file has a size that it is unique, it is safe to assume that it is unique and does not need to be included in the set of files I am checking for duplicates. Additionally, I must only consider files that are listed in the csv. Occasionally the csv has filenames of files that are not in the directory, which leaves me with several choices.

1. I can initially check my list of files for those which do not exist and remove them. This adds a loop over all the files and seems unnecessary/inefficient; it is very rare that a file does not exist.
2. Use the following code with exception handling when the file is not found and delete file from my list at this point. However this introduces a side effect to the grouping function. Clearly this function does more than its name suggests.

Code:

def group_by_size(self, files_to_group):
    grouped_by_size = dict()
    for fileid in files_to_group:
        full_path = os.path.join(self.folder, fileid)
        try:
            size = os.path.getsize(full_path)
        except WindowsError:
            print WindowsError
            print "The file wasn't found. Deleting row from PremsPhotos."
            del self.files[fileid]
            del self.spreadsheet_reader.main_row_dict[fileid]
        if grouped_by_size.get(size):
            grouped_by_size[size].append(fileid)
        else:
            grouped_by_size[size]=[]
            grouped_by_size[size].append(fileid)
    return grouped_by_size

Alternatively, eliminate duplicates beforehand:

    def remove_nonexistant_photos(self):
        for photo in self.files:
            os.path.exists(os.path.join(self.folder, photo))

The context of this code is in a script that will likely be run a lot, but will not likely be modified once it works. Looping over all the files in the directory takes about half a second when there are 15k files in the directory. With both of these in mind, I suppose the decision is not of huge importance, but I have been doing a lot of reading about best practices, and want to improve my coding intuition/style as I go forward.

Answer 1

The group_by_size function has multiple responsibilities:

• Group files by size
• Verify file
• Modify attributes of the class (self.files, self.spreadsheet_reader.main_row_dict)

It would be better to reorganize your code so that each function has a single responsibility.

---

There is some code duplication here that could be improved:

if grouped_by_size.get(size):
    grouped_by_size[size].append(fileid)
else:
    grouped_by_size[size]=[]
    grouped_by_size[size].append(fileid)

Like this:

files = grouped_by_size.get(size)
if files is None:
    files = []
    grouped_by_size[size] = files
files.append(fileid)