My Python Program Files cleaner, a better way to write this?

Question

Similar to my other post I have created a new little utility and wanted to get some feedback if there is a way to make my script more Pythonic per say. It is a simple tool for Windows that removes empty directory from the Program Files directory because sometimes programs leave behind an empty directory when they uninstall.

import os

def path_exist(path):
    return os.path.exists(path)

def list_dir(path):
    return os.listdir(path)

def is_dir(path):
    return os.path.isdir(path)

def main():
    x86_64 = 'C:\\Program Files\\'
    x86 =  'C:\\Program Files (x86)\\'

    if path_exist(x86_64):
        for i in list_dir(x86_64):
            i = x86_64 + i
            if is_dir(i):
                if not list_dir(i):
                    os.rmdir(i)
                    print 'Removed', i

    if path_exist(x86):
        for i in list_dir(x86):
            i = x86 + i
            if is_dir(i):
                if not list_dir(i):
                    os.rmdir(i)
                    print 'Removed', i

if __name__ == '__main__':
    main()

Answer 1

I would try to avoid repetition of the for i in list_dir(something) doing something like:

if path_exist(x86_64):
    path = x86_64
else if path_exist(x86):
path = x86
else:
exit(1)

for i in list_dir(path):
  i = x86_64 + i
  if is_dir(i):
if not list_dir(i):
  os.rmdir(i)
  print 'Removed', i

Answer 2

Some remarks:

a) I find this sort of thing clutters up the script and I don't see the advantage of using path_exists() instead of os.path.exists(). But hey.

def path_exist(path):
    return os.path.exists(path)

def list_dir(path):
    return os.listdir(path)

def is_dir(path):
    return os.path.isdir(path)

b) More seriously, I'd suggest you use os.path.join() to connect the pat components instead of simple concatenation.

c) I don't think i is a good variable name for a directory listing element. Why not:

for element in list_dir(path):
  newpath = os.path.join(x86_64, element)

EDIT: Even better -- it slipped out of my mind -- Triptych is entirely correct that you should be using os.walk() in the first place.

d) My personal preference would be to use try...except blocks to check whether you can delete a directory -- "asking for forgiveness, instead of permission". You're practicing "look before you leap".

e) For general style advice, don't forget to put PEP8 under your pillow!

Answer 3

1. The 3 functions at the beginning of the file seem pretty useless, and simply wrap os functions. I'd change that to from os.path import exists etc.
2. You can basically extract a function out of the two loops that are exactly the same - simply call it remove_empty_dirs or something, and make it receive the path as an argument. That will keep you DRY.
3. The path variables can be changed to a list of paths, which you can then iterate through and provide to the new function created in step 2
4. It will be more readable not to change i while using it to loop, but simply using a new variable
5. In general i isn't that good a name, such as sub_dir etc.

Answer 4

1. Use os.walk
2. Create a function for common code
3. Don't write wrappers around names if you're not adding any new functionality

A rewrite:

import os

def kill_empty_dirs(directory):
   """ Deletes directories with no files and no subdirectories """

   for dir, subdirs, files in os.walk(directory):
      if not subdirs and not files:
         print 'DELETING: ', dir 

Obviously, replace my print line with a real deletion after testing.

Answer 5

A less radical refactoring than Triptych's

import os

def main():
    x86_64 = 'C:\\Program Files\\'
    x86 =  'C:\\Program Files (x86)\\'

    for base in x86_64, x86:
        if not os.path.exists(base):
            continue
        for diritem in os.listdir(base):
            path = os.path.join(base, diritem) 
            if os.path.isdir(path) and not os.listdir(path):
                os.rmdir(path)
                print 'Removed', path

if __name__ == '__main__':
    main()

Answer 6

You can replace the first three subroutines with:

from os.path import exists as path_exist
from os import listdir as list_dir
from os.path import isdir as is_dir

of course, none of those are needed if you follow Triptych's suggestion and use os.walk(), which is clearly a better solution.