Code Review

Peter Otten __peter__ at web.de
Wed May 25 03:22:31 EDT 2011


ad wrote:

> Please review the code pasted below. I am wondering what other ways
> there are of performing the same tasks. This was typed using version
> 3.2. The script is designed to clean up a directory (FTP, Logs, etc.)
> Basically you pass two arguments. The first argument is an number of
> days old to delete. The second argument is the directory where the
> files and folders should be deleted. I imagine one enhancement would
> be to create a function out of some of this.

> CurrentTime = time.time()

Read PEP 8 on naming conventions etc., see 
http://python.org/dev/peps/pep-0008/
> 
> epocDay = 86400   # seconds
> 
> 
> 
> 
> 
> parser = argparse.ArgumentParser(description = "Delete files and

What's the purpose of those many empty lines?

> folders in a directory N days old", add_help=False,
> prog='directorycleaner', usage='%(prog)s 7 c:\\temp')
> 
> parser.add_argument('days', type=int, help="Numeric value: delete
> files and folders older then N days")
> 
> parser.add_argument('directory', help="delete files and folders in
> this directory")
> 
> parser.print_help()

What's the idea behind add_help=False and the explicit print_help()?

> dictKeys = (vars(args))
> HowManyDays = dictKeys['days']

This can be simplified to

HowManyDays = args.days

> if dirExists == False: print ("The directory is missing")

if x == False: ...

is normally written as 

if not x: ...

> DirListing = os.listdir(WhatDirectory)
> for files in DirListing:

You might write this as

for filename in os.listdir(WhatDirectory): ...

or even

for filename in os.listdir(args.directory): ...

Personally I would put this stuff in a separate function like

def remove_old_files_and_folders(parent_directory, age_in_seconds):
    ...

>     # time.ctime converts epoch to a normal date
> 
>     #print (time.ctime(CurrentTime))
> 
>     # Get the date from seven days ago
> 
>     WeekOldFileDate = CurrentTime - DaysToDelete
> 
>     #print (CurrentTime)
> 
>     #print (FileCreationTime)
> 
>     #print (WeekOldFileDate)

Don't let out-commented code eclipse the actual code; remove it. If you want 
to preserve it for eternity, have a look at version control systems, e. g. 
Mercurial.




More information about the Python-list mailing list