[Fwd: Re: [plt-scheme] Scheme Program]
Dear Matt and Danny,
Oh dear, I am really sorry, I have received Danny's email but have
missed it as I do get a lot of emails! I have had a flue since the
weekend and still trying to recover. I've been trying to work from my
bed, but not doing a good job :-(
I am not an expert programmer but I'm learning and I do appreciate
your detailed analysis and your time for helping. I will go through it
in detail and will reply back if you don't mind please.
Thank you very much.
HZBilal.
---
Mr Haider Zuhair Bilal BEng MSc MIEE MIEEE MBCS
PhD Research Scholar
Centre for Systems and Software Engineering
Faculty of Business, Computing and Information Management
London South Bank University
103 Borough Road
London SE1 0AA
UK
Tel: +44(0)20 7815 7473
Fax: +44(0)20 7815 7550
Mobile: +44(0)7887 598355
http://myweb.lsbu.ac.uk/~bilalhz/
-----Original Message-----
From: Matt Jadud [mailto:mcj4 at kent.ac.uk]
Sent: 11 October 2006 09:57
To: Bilal, Haider Z
Cc: Danny Yoo
Subject: [Fwd: Re: [plt-scheme] Scheme Program]
Hi Bilal,
Danny gave you a good, thoughtful reply just a few days ago. If you're
subscribed to the list, and you're asking questions to the list, you
should watch for replies to your messages.
Danny has already outlined a number of steps you can take, and I
suggest
you take everything he said on-board. His advice is, in my experience,
usually quite good. When it isn't "quite good", it's bordering on
"excellent". If that seems over-the-top, then I'll claim that the word
"thorough" always applies to the answers he provides on-list.
Cheers,
Matt
-------- Original Message --------
Subject: Re: [plt-scheme] Scheme Program
Date: Mon, 2 Oct 2006 14:29:16 -0700 (PDT)
From: Danny Yoo <dyoo at hkn.eecs.berkeley.edu>
To: Bilal, Haider Z <bilalhz at lsbu.ac.uk>
CC: plt-scheme at flux.utah.edu
References:
<9FFEE39DADA4A442B06FB5673765F41002847F77 at CSD-EXBE-VS1.lsbu.ac.uk>
> However, this section of the code is not working properly and giving
me
> the following error: apply: bad procedure: #[undefined]
Hi Haider,
The following is a code review of the code. Please don't take any of
the
comments as condemnation against your own person. I'll try to keep
criticism directed at the code.
Code review comments:
* I tried reading your program. I suppose I'm not be a Schemer
yet,
but I think the program is, frankly, hard to read. It's way
too
nested for me to read comfortably, and the indentation is in
places
that does not seem natural. Double-spacing Scheme code is not
common.
* There are superfluous expressions. Doing a test like:
(not (equal? (pdg-vertex-ids-killed v) #f))
can be more simply expressed as:
(pdg-vertex-ids-killed v)
* There's a LOT of mutation going on in the code. Are you sure
you
want to do that? It's really making the code more complicated
than
it needs to be.
* There are subexpressions that basically look like big
copy-and-paste
blocks. Don't copy-and-paste: abstract! Concretely, the only
difference I see between
(if (pdg-vertex-ids-killed v)
(abs-loc-set-traverse
(pdg-vertex-ids-killed v)
(lambda (y)
(let*
((line-num
(file-get-line-num fileuid offset)))
(set! linelist
(cons (list line-num y v)
linelist)))
#t)))
and:
(if (pdg-vertex-ids-used v)
(abs-loc-set-traverse
(pdg-vertex-ids-used v)
(lambda (y)
(let*
((line-num
(file-get-line-num fileuid offset)))
(set! linelist
(cons (list line-num y v)
linelist)))
#t)))
is just one minor subexpression.
* Everything seems to be stuffed in one gargantuan function.
Consider
writing helper functions where it will clarify the code's
intent.
* I see no direct use of APPLY in your code, so something else
must be
calling it. Actually, I see a few type errors in the code.
Concretely: what's the "contract" of ABS-LOC-SET-TRAVERSE?
Check
your usages of that function: the code is definitely violating
its use of
ABS-LOC-SET-TRAVERSE in a few places.
You made the comment:
> However, it gave me wrong output, some had TO and FROM both listed
> together:
What in the program prevents from happening? From the sequence of
subexpressions:
(if (pdg-vertex-ids-killed (caddr z))
(abs-loc-set-traverse ...))
(if (pdg-vertex-ids-used (caddr z))
(abs-loc-set-traverse ...))
there's nothing in the code that excludes both PDG-VERTEX-IDS-KILLED
and
PDG-VERTEX-IDS-USED to return true values.
If you are going to write more Scheme code, you might also want to
consider doing a read-through a textbook like HTDP:
http://htdp.org
If you do so, your programs will tend to have a better design.
I've reformatted your code as best as I can in the context of a PLT
Scheme
module, to be able to use the Syntax Check tool as well as better see
the
structure of the program.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;;;;;;;
(module pdg mzscheme
;; Unknown: the following definitions are here only so I can run
;; check syntax and see where values are coming from.
(define (pdg-vertex-set-traverse l f) (void))
(define (int-pair-set-traverse l f) (void))
(define (abs-loc-set-traverse l f) (void))
(define (pdg-vertices pdg) (void))
(define (pdg-compilation-uid pdg) (void))
(define (pdg-vertex-pdg pdg) (void))
(define (pdg-kind pdg) (void))
(define (pdg-vertex-charpos pdg) (void))
(define (pdg-vertex-ids-killed pdg) (void))
(define (pdg-vertex-ids-used pdg) (void))
(define (file-get-line-num x y) (void))
(define (sort l cmp) (void))
(define (sdg-pdgs) (void))
(define (assignments)
(define filenumber 1)
(define (process-pdg pdg)
(when (eqv? (pdg-kind pdg) 'user-defined)
(let* ((out-port
(open-output-file
(string-append "Fun"
(number->string filenumber)
"_V.dat")))
(linelist ()))
(pdg-vertex-set-traverse
(pdg-vertices pdg)
(lambda (v)
(let* ((fileuid
(pdg-compilation-uid
(pdg-vertex-pdg v))))
(int-pair-set-traverse
(pdg-vertex-charpos v)
(lambda (offset w)
(if (pdg-vertex-ids-killed v)
(abs-loc-set-traverse
(pdg-vertex-ids-killed v)
(lambda (y)
(let*
((line-num
(file-get-line-num
fileuid offset)))
(set! linelist
(cons (list line-num y v)
linelist)))
#t)))
(if (pdg-vertex-ids-used v)
(abs-loc-set-traverse
(pdg-vertex-ids-used v)
(lambda (y)
(let*
((line-num
(file-get-line-num
fileuid offset)))
(set! linelist
(cons (list line-num y v)
linelist)))
#t)))
#t)))
#t))
(set! linelist
(sort linelist
(lambda (a b)
(< (cadar a) (cadar b)))))
(for-each (lambda (z)
(let
((linenum (car z)))
(define line-num (car z))
(display (cdr line-num) out-port)
(write-char #\tab out-port)
(if (pdg-vertex-ids-killed (caddr z))
(abs-loc-set-traverse
(pdg-vertex-ids-killed (caddr z))
(display "to" out-port)))
(if (pdg-vertex-ids-used (caddr z))
(abs-loc-set-traverse
(pdg-vertex-ids-used (caddr z))
(display "from" out-port)))
(newline out-port)))
linelist)
(close-output-port out-port))
(set! filenumber (+ filenumber 1))))
(for-each process-pdg (sdg-pdgs)))
(assignments))
_________________________________________________
For list-related administrative tasks:
http://list.cs.brown.edu/mailman/listinfo/plt-scheme
This e-mail message may be confidential and is intended only for the use of the individual(s) to whom it is addressed. It may contain information which is or may be confidential, non-public or legally privileged. Please do not disseminate or distribute this message other than to its intended recipient without permission of the author. You should not copy it or use it for any purpose nor disclose its contents to any other person. If you have received this message in error, please notify me by email immediately and delete the original message and all copies in your computer systems.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.racket-lang.org/users/archive/attachments/20061012/eef7cfbc/attachment.html>
![[logo]](/logo.png){kind=logo}