Code Review

[Ulrich Eckhardt]

ad wrote:
> Please review the code pasted below. I am wondering what other ways
> there are of performing the same tasks.

On a unix system, you would call "find" with according arguments and then 
handle the found files with "-exec rm ..." or something like that, but I see 
you are on MS Windows.


> args = parser.parse_args()
> 
> dictKeys = (vars(args))

The first of these looks okay, while I don't get the additional brackets in 
the second one. Another habit I observe here is the Hungarian notation of 
prefixing the type to the name and using camelCaps. PEP 8 (IIRC) has 
something to say on the preferred naming. I'm not 100% against encoding the 
type in the variable name in Python, since it lacks static type checking, I 
would have chosen "key_dict" here though, or, due to the small size of the 
overall program just "keys".

> print (HowManyDays)

This puzzled me at first, again the useless additional brackets I thought. 
However, in Python 3, "print" is a function, so that is correct. Still, it 
should be "print(foo)" not "print (foo)".

> for files in DirListing:
> 
>     # Get the absolute path of the file name
>     abspath = (os.path.join(WhatDirectory, files))

"files" is just the name of a single file, right? In that case the name is a 
bit confusing.

>     # Get the date from seven days ago
>     WeekOldFileDate = CurrentTime - DaysToDelete

You are repeating this calculation for every file in the loop.

>     if FileCreationTime < WeekOldFileDate:
>         #check if the object is a file
>         if os.path.isfile(abspath): os.remove(abspath)
>         # It is not a file it is a directory
>         elif os.path.isdir(abspath): shutil.rmtree(abspath)

I'm not sure, but I believe you could use shutil.rmtree() for both files and 
directories. In any case, be prepared for the file still being open or 
otherwise read-only, i.e. for having to handle errors.

Also, what if a directory is old but the content is new? Would this cause 
the non-old content to be deleted?


Cheers!

Uli


-- 
Domino Laser GmbH
Geschäftsführer: Thorsten Föcking, Amtsgericht Hamburg HR B62 932