Hi Marjin,<div><br></div><div>Thanks for your effort here, it's much appreciated and I prefer your solution to mine in every meaningful way.</div><div><br></div><div>I've converted your solution to work with Gambit.  There were only four small changes, but I've had to lose the steroid powered match-lambda* and replace it with a simple  cond.</div>
<div><br></div><div>I've got Meroon built into this app, and I'll write the class based equivalent to see what it looks like.  However I suspect that it will be more clumsy for something this straightforwards and probably isn't the best solution unless you need to make use of a feature like inheritance.</div>
<div><br></div><div><br></div><div>Roger.</div><div><br></div><div><br><div><br><br><div class="gmail_quote">On Thu, Jan 19, 2012 at 3:17 PM, Marijn <span dir="ltr"><<a href="mailto:hkBst@gentoo.org">hkBst@gentoo.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">-----BEGIN PGP SIGNED MESSAGE-----<br>
Hash: SHA1<br>
<br>
On 19-01-12 01:16, Roger Wilson wrote:<br>
> Hi,<br>
<br>
Hi Roger<br>
<div class="im"><br>
> I'm still using Gambit Scheme from time to time and having a lot of<br>
> fun with it.  I've a newbie question however, I'm trying to figure<br>
> out the best way to structure code. With that in mind are there any<br>
> comments on the following code?<br>
<br>
</div>This is often a good question, so since you asked I've spent some time<br>
tearing your code apart, but also preparing an alternative answer, so<br>
bear with me.<br>
<div class="im"><br>
> I'm trying to create an 'object' that holds a list of names, can<br>
> return a random one on request that can later be returned to it.<br>
<br>
</div>You mention a list here, but really that is not the right data<br>
structure for random access, so don't take it literally.<br>
<div class="im"><br>
> What's the proper Scheme way of doing this kind of thing?  Here's<br>
> my stab, it returns a function that then accepts 'messages' as<br>
> parameters....<br>
<br>
</div>My solution will follow you here although an alternative is to use an<br>
object/class system depending on your scheme implementation so you<br>
don't have to wire public methods manually.<br>
<div class="im"><br>
> (define (make-name-list seed-names)<br>
><br>
> ;; The type the list is built from (define-type blob-name<br>
<br>
</div>Really dislike the name `blob-name' you chose here and it leads to<br>
horrendously named accessors. Better just use `blob', `entry' or<br>
`elt'. Also there is no reason this data structure need only contain<br>
names; really any scheme value can be stored in it, so you should<br>
choose more generic names based on that.<br>
<div class="im"><br>
> name (random init: 0))     ; Used for sorting<br>
<br>
</div>Such as `item' and `key'.<br>
<br>
> (let ((unused-name-list '()))<br>
<br>
Why would you want to have an ``unused list'' in your data structure?<br>
Surely it would be better if the data structure members are actually<br>
put to good use? ;P<br>
<div class="im"><br>
> (define (randomise-list) ; Assign each name a random number and<br>
> then sort them (map (lambda (x) (blob-name-random-set! x<br>
> (random-integer 65536))) unused-name-list)<br>
<br>
> (sort! unused-name-list (lambda (x y) (if (< (blob-name-random x)<br>
> (blob-name-random y)) #t #f))))<br>
><br>
><br>
> ;; Put all the seed names in the list (map (lambda (x) (push<br>
> (make-blob-name x) unused-name-list)) seed-names)<br>
<br>
</div>Try to limit the line-width you use (for email consumption).<br>
<div class="im"><br>
> (lambda (message . arguments) (cond ((eq? message ':get-name!) (if<br>
> (null? unused-name-list) "No names available" (begin (let ((name<br>
> (blob-name-name (car unused-name-list)))) (set! unused-name-list<br>
> (cdr unused-name-list)) name))))<br>
><br>
> ; Add a name to the list, used to return names once they're no<br>
> longer needed<br>
<br>
</div>What do you mean no longer needed? These names WILL come back if you<br>
':get-item! long enough.<br>
<div class="im"><br>
> ((eq? message ':put-name!) (push (make-blob-name (car arguments))<br>
> unused-name-list) (randomise-list))<br>
<br>
<br>
> ; Debug, print the internal display ((eq? message ':display)<br>
> (display unused-name-list))<br>
><br>
> (else (error 'name-list "unrecognized message"))))))<br>
><br>
><br>
> ;; Test drive code....... (define name-list (make-name-list (list<br>
> "Albert" "Bruce" "Clive"))) (display (name-list ':get-name!))<br>
> (name-list ':put-name! "David") (name-list ':put-name! "Edgar")<br>
> (name-list ':put-name! "Frederick")<br>
><br>
> (display (name-list ':display))<br>
<br>
</div>Excellent, we have test code!<br>
<div class="im"><br>
> Some points:<br>
><br>
> In particular I'm unhappy with the way the message arguments are<br>
> accessed as (car arguments) rather than by explicit name.  I can<br>
> see that being a potential cause of bugs if the number of arguments<br>
> grows and they vary with each message.<br>
<br>
</div>Really what you would like here is to be able to match the arguments<br>
to a number of patterns which correspond to the different ways this<br>
procedure can be validly called. Thinks of `match' as `case' on<br>
steroids. Really BIG steroids. Steroids so big you can hide several<br>
dozen cases of `case's in their shadow. `match' swallows these<br>
steroids whole so you get the idea...<br>
A quick search seems to indicate `no', but: Does gambit have `match'?<br>
<div class="im"><br>
> I realise that the randomise-list function is probably called too<br>
> often and maintaining a 'dirty bit' would allow it to be only<br>
> called when necessary when getting a name.<br>
<br>
</div>It is an option, but you can do even better if you use a vector to<br>
store your items.<br>
<div class="im"><br>
> I've years of C/C++ programming behind me, which is probably<br>
> readily apparent.<br>
<br>
</div>No comment? ;P<br>
Seriously, you made that too easy.<br>
More seriously, it can go either way and if you're aware of the<br>
potential for bad influence (like you seem to be) then it should be<br>
possible to use that experience to your advantage. For example when<br>
estimating the cost of certain constructs.<br>
<div class="im"><br>
> How can I improve the code?<br>
<br>
</div>I have prepared an alternative solution to your specification here. In<br>
racket since that is what I am most familiar with atm. Really the only<br>
racket-specific stuff seems to be the match-lambda*, although keyword<br>
arguments probably work differently in gambit and I'm not sure gambit<br>
has a vector->values which isn't essential to anything. And I don't<br>
know about unit testing for gambit. Some expert gambitteer(?) should<br>
be able to fill in the details of those differences.<br>
<br>
<br>
#lang racket<br>
<br>
(require rackunit)<br>
<br>
(define (make-random-store #:capacity (capacity 99) . init-entries)<br>
  (define size 0)<br>
  (define store (make-vector capacity))<br>
<br>
  (define (add-entries entries)<br>
    (for-each (lambda (e) (add-entry e)) entries))<br>
<br>
  (define (add-entry entry)<br>
    (define (_add-entry)<br>
      (vector-set! store size entry)<br>
      (set! size (+ size 1)))<br>
<br>
    (define capacity (vector-length store))<br>
<br>
    (if (< size capacity)<br>
        (_add-entry)<br>
        (let ((new-store (make-vector (* 2 capacity))))<br>
          (vector-copy! new-store 0 store)<br>
          (set! store new-store)<br>
          (_add-entry)))  )<br>
<br>
  (define (remove-entry n)<br>
    (cond ((< -1 n size)<br>
           (set! size (- size 1))<br>
           (define ret (vector-ref store n))<br>
           (vector-set! store n (vector-ref store size))<br>
           ret)<br>
          (else (error "index out of range")) )  )<br>
<br>
  (define (entries)<br>
    (vector->values store 0 size))<br>
<br>
  (add-entries init-entries)<br>
<br>
  (match-lambda*<br>
    ((list 'put! entries ...) (add-entries entries))<br>
    ((list 'get!) (remove-entry (random size)))<br>
    ((list 'size) size)<br>
    ((list 'entries) (entries)) )  )<br>
<br>
;; Test drive code.......<br>
(define name-list (make-random-store "Albert" "Bruce" "Clive"))<br>
(check-true<br>
 (set=? (call-with-values (lambda () (name-list 'entries)) set)<br>
        (set "Albert" "Bruce" "Clive")))<br>
(define name1 (name-list 'get!))<br>
(check-not-false (member name1 '("Albert" "Bruce" "Clive")))<br>
(name-list 'put! "David" "Paul")<br>
(name-list 'put! "Edgar")<br>
(name-list 'put! "Frederick")<br>
(define name2 (name-list 'get!))<br>
(check-true<br>
 (set=?<br>
  (set-union<br>
   (set name1 name2)<br>
   (call-with-values (lambda () (name-list 'entries)) set))<br>
  (set "Albert" "Bruce" "Clive" "David" "Paul" "Edgar" "Frederick") ))<br>
<br>
<br>
I hope that was helpful,<br>
<br>
Marijn<br>
-----BEGIN PGP SIGNATURE-----<br>
Version: GnuPG v2.0.18 (GNU/Linux)<br>
Comment: Using GnuPG with Mozilla - <a href="http://enigmail.mozdev.org/" target="_blank">http://enigmail.mozdev.org/</a><br>
<br>
iEYEARECAAYFAk8YM/cACgkQp/VmCx0OL2w66ACfQXj/PeGIf0Ibq3AdebTmVKiD<br>
cwQAoJf/Uz9hRyK1ekPcjJfVbTGm09jS<br>
=XdPT<br>
-----END PGP SIGNATURE-----<br>
</blockquote></div><br></div></div>