Code Review

[ad]

On May 25, 4:06 am, Ulrich Eckhardt <ulrich.eckha... at dominolaser.com>
wrote:
> 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

Thank you guys very much for the excellent points. I will use this
information as a reference as I write more code and fix up the
existing script.

Chris, thank you for putting so much time into your post!

Until we type again...........