Path: csiph.com!x330-a1.tempe.blueboxinc.net!usenet.pasdenom.info!aioe.org!feeder.news-service.com!feeder3.cambriumusenet.nl!feed.tweaknews.nl!193.201.147.92.MISMATCH!xlned.com!feeder5.xlned.com!newsfeed.xs4all.nl!newsfeed6.news.xs4all.nl!xs4all!newsgate.cistron.nl!newsgate.news.xs4all.nl!post.news.xs4all.nl!not-for-mail
To: python-list@python.org
From: Peter Otten <__peter__@web.de>
Subject: Re: Code Review
Date: Wed, 25 May 2011 09:22:31 +0200
Organization: None
References: <37ba7b40-3663-4094-b507-696fc598bf48@l26g2000yqm.googlegroups.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="ISO-8859-1"
Content-Transfer-Encoding: 7Bit
Precedence: list
Newsgroups: comp.lang.python
Message-ID: <mailman.2054.1306308167.9059.python-list@python.org>
Lines: 87
NNTP-Posting-Host: 82.94.164.166
Xref: x330-a1.tempe.blueboxinc.net comp.lang.python:6198

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.