Code Review

[Iain King]

On May 25, 2:44 pm, ad <adsquai... at gmail.com> wrote:
> 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...........


Wrote something to do the same basic thing a little while ago.  Less
verbose than yours, and it only does files, not folders.  If I was
going to do folders though, I'd do them by recursing into them and
pruning files, and not go by the folder modify date, which I don't
think changes the way you think it changes (for example, if a file
inside a folder got updated such that it shouln't be deleted it still
will be with your code if the folder modify date is old [this is on
Windows])

import os, glob, time, sys

if len(sys.argv) < 3:
    print "USAGE: %s <PATTERN> <DAYS>" % sys.argv[0]
    sys.exit(1)

pattern = sys.argv[1]
days = int(sys.argv[2])
threshold = days * 24 * 60 * 60
t = time.time()

for f in glob.glob(pattern):
    if t - os.stat(f)[9] > threshold:
        print f
        os.remove(f)