scsh-users
[Top] [All Lists]

Re: Code critique request

To: seanwinship@yahoo.com (Sean Winship)
Subject: Re: Code critique request
From: Martin Gasbichler <gasbichl@informatik.uni-tuebingen.de>
Date: Tue, 22 Jun 2004 09:51:53 +0200
Cc: scsh-users@scsh.net
List-id: <scsh-users.list-id.scsh.net>
seanwinship@yahoo.com (Sean Winship) writes:

> I wrote the following script to track inactive users on one of our
> machines.  I have modified it as necessary so as not to identify my
> employer; hopefully it still makes sense.  We have hardware available
> internally for "skunkworks" use.  Employees can get probationary
> access which will be revoked if they aren't actively using the
> resources.  Anyone with an active project will not be considered
> inactive, even if they are still listed as probationary.
>
> I've been learning Scheme on my own and have no one at work to discuss
> it with.  Any comments or critiques of this script would be
> appreciated.  I'm particularly concerned about my use of setf!, which
> seems to go against the spirit of Scheme that I've picked up from
> SICP.

Yes, the use of SET! in COUNT-RECENT-FILES is the only thing that
looks a little bit odd. Mutating local variables should be avoided
whenever possible as it makes the code much harder to understand.

I'd rewrite the function as follows (untested)

(define (count-recent-files base-dir since-date)
  (if (and (file-exists? base-dir)
           (file-directory? base-dir))
      (with-cwd base-dir
                (let lp ((count 0)
                         (files (directory-files)))
                  (if (null? files)
                      count
                      (let ((file (car files)))
                        (lp 
                         (if (file-directory? file)
                             (+ count
                                (count-recent-files
                                 file
                                 since-date))
                             (if (< since-date (file-last-mod file))
                                 (+ 1 count)
                                 count))
                         (cdr files))))))
      #f))

A slightly more advanced version uses FOLD-RIGHT from SRFI-1 (BTW: I'd
recommend to use srfi-1 instead of list-lib). FOLD-RIGHT frees you
from typing the boring list recursion pattern over and over again
(untested as well):

(define (count-recent-files base-dir since-date)
  (if (and (file-exists? base-dir)
           (file-directory? base-dir))
      (with-cwd base-dir
                (fold-right
                 (lambda (file count)
                   (if (file-directory? file)
                       (+ count
                          (count-recent-files
                           file
                           since-date))
                       (if (< since-date (file-last-mod file))
                           (+ 1 count)
                           count)))
                 0
                 (directory-files)))
      #f))


-- 
Martin

<Prev in Thread] Current Thread [Next in Thread>