chicken-users
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Chicken-users] Review my Caesar Cipher?


From: Jörg F. Wittenberger
Subject: Re: [Chicken-users] Review my Caesar Cipher?
Date: Mon, 10 Mar 2014 16:40:45 +0100
User-agent: Mozilla/5.0 (X11; Linux armv7l; rv:17.0) Gecko/20131104 Icedove/17.0.10

Am 10.03.2014 15:51, schrieb Daniel Carrera:
Hello,

I found a Scheme implementation of the Caesar cipher on Rosetta Code. It said "This was written by a novice, please review"... So I reviewed it, and basically rewrote it.

I think my version is much better (clearer)

I tend to agree that your version is clearer.  Especially because it's shorter and uses fewer let-scopes.

As this is an exercise, I'd take an issue with the use of list-ref in the replacements.  List-ref is O(n) in the size of the list.  Once should try to avoid that.

A first alternative would be string-ref.  That would be O(1).  As long as your Scheme does not use something like UTF-8 as string representation.  So maybe it would be the best to resort to something like:

(define replacements (apply vector (string->list rotated)))
 
(define (caesar char)
  (let ((index (string-index alphabet char)))
    (if index
        (vector-ref replacements index)
        char))) ; Not found => Copy verbatim.

Notice: I replaced the second occurrence of "(string-index alphabet char)" with "index" - the variable the result was of the first call was already bound to.  I'm leaving it here as an exercise to you to figure out why.  ;-)


Your second version brings up a completely different consideration.  The task "implement a caesar chipher" is slightly underspecified.  That is, actually it's OK, since it would imply that you are supposed to produce a general solution.  And your first version does.

You second version depends on the mapping from characters to integers.  It will "only" work on such mappings, which are "accidentally" compatible to ASCII for upper case letters.  I'm using quotes here, because the days are long gone, when you had a reasonable chance to get your hands on a system using incompatible encodings like
http://en.wikipedia.org/wiki/DEC_Radix-50
Otherwise I'd prefer the second version for using less memory.

Note however: if you wanted the your cipher to be easily adapted to more general mappings (e.g. be applicable to other character sets than upper case ASCII compatible - which would easily be parts of unicode or say HTML entities like ü) then the argument is reversed and your first version would be the better fit.

but since I too am a novice, I feel bad removing the "novice" warning. Could someone who has used Scheme longer than two weeks have a quick look at my work and tell me if I can remove the warning (or make corrections if my code is not idiomatic or something)?

My version(s):


Previous version:


Cheers,
Daniel.
--
When an engineer says that something can't be done, it's a code phrase that means it's not fun to do.


_______________________________________________
Chicken-users mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/chicken-users


reply via email to

[Prev in Thread] Current Thread [Next in Thread]