5
\$\begingroup\$

Just looking for some feedback. I designed a script to find big files that are taking up lots of space. Still a beginner, and appreciative of constructive feedback, with both the function and form/style/formatting.

#! /usr/bin/env python3
# -*- coding: utf-8 -*-
# findlargefiles.py Searches a file location and subdirectories for
# files larger than a given size.
"""
findlargefiles.py Searches a file location and subdirectories for
files larger than a given size.

Created on Sun Sep  3 20:35:12 2017

@author: toonarmycaptain
"""

import os


def searchfolder(location, filesize):
    FileNotFoundErrorsCount = 0
    filesfoundcount = 0
    print('Files larger than '+str(int(filesize))+' MB in location: '+location)
    for foldername, subfolders, filenames in os.walk(location):
            for filename in filenames:
                try:
                    actualsize = os.path.getsize(os.path.join(foldername,
                                                              filename))
                    if filesize*1024**2 <= actualsize:
                        print(foldername
                              + '\\'  # file path + \ + file name
                              + filename
                              + ' - '
                              + '{:.2f} MB'.format(actualsize/1024**2))
                        filesfoundcount += 1
                except FileNotFoundError:
                    FileNotFoundErrorsCount += 1
                    print('FileNotFoundError: '+filename)
    print('Files found: '+str(filesfoundcount))
    if FileNotFoundErrorsCount > 0:
        print('FileNotFoundErrors: '+str(FileNotFoundErrorsCount))


print('This program searches for files larger than a given size '
      'in a given location.')

while True:
    location = input("Where would you like to search? ")
    if os.path.exists(location):
        break
    else:
        print('Please enter a valid path.')
filesize = float(input('Please enter file size in MB: '))

searchfolder(location, filesize)
\$\endgroup\$
5
\$\begingroup\$

A more flexible approach would separate the printing from the finding. This allows you to use search_folder for other purposes, and cleans up the logic a bit.

More idiomatically in Python this might be formatted something like:

def search_folder(location, min_filesize):
    for folder, subfolders, filenames in os.walk(location):
        for filename in filenames:
            try:
                size_bytes = os.path.getsize(os.path.join(foldername, filename))
                if min_filesize * 1024 ** 2 <= size_bytes:
                    yield filename, size_bytes
            except FileNotFoundError:
                # maybe log error, maybe `pass`, maybe raise an exception
                # (halting further processing), maybe return an error object

if __name__ == '__main__':
    print('This program searches for ...')
    ...

    print('Files larger than %d MB in location: %s' % (filesize, location))
    for filename, size in search_folder(location, filesize):
        print(...)
  • The if __name__ == '__main__' block allows you to import this script in other Python files, without triggering the interactive logic.
  • search_folder now returns an iterator of (filename, size) pairs, and leaves the printing up to the function that calls it.
    • Now I can run this with no printing, purely to count files, for example.
  • Standard Python style is to use underscore_case for function and variable names, and especially to start with lowercase (eg. files_not_found_count instead of FileNotFoundErrorsCount)
  • Wrapping things in str() when printing is unnecessary if you use one of the various string-formatting tools (%-formatting or '{}'.format) instead of +
  • I'm not sure under what conditions you would get FileNotFoundError on paths returned from os.walk, other than deleted files. If that's true in general, the most appropriate error handling may just be to silently continue.
\$\endgroup\$
5
  • \$\begingroup\$ Thankyou so much - I hadn't learned about importing code I'd written yet, but I tried importing it and it worked! (Pretty exciting for me). \$\endgroup\$ – toonarmycaptain Sep 4 '17 at 14:11
  • \$\begingroup\$ I had already used .format() once, I don't know why I didn't do it for the rest. Fixed now. \$\endgroup\$ – toonarmycaptain Sep 4 '17 at 14:12
  • \$\begingroup\$ The error was originally happening because I wasn't getting the file paths right for subfolders, and at the time I thought it was for hidden files or something(like it could see names, but not sizes), because the script returns a bunch of files I don't see in .File Explorer. \$\endgroup\$ – toonarmycaptain Sep 4 '17 at 14:12
  • \$\begingroup\$ Am I right in following that what you're doing to avoid the print statement is to use the yield statement, and duplicating what was originally there, with the print statements inside if __name__ == '__main__':? \$\endgroup\$ – toonarmycaptain Sep 4 '17 at 14:16
  • \$\begingroup\$ @toonarmycaptain Yup. I just moved your input loop and all the print statements into the if __name__ block, and gave it its own loop (for filename, size in search_folder(..)) that will receive the filename, size pairs that search_folder has found to be over the min size. \$\endgroup\$ – BenC Sep 4 '17 at 21:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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