Hello everyone!
I have spent the last months studying Lisp and playing with SLIME.
I come up with the following two toy programs and I'd like to have
them reviewed.
In particular, I'd like to know whether the code is somewhat lispy, if
it shows that I learnt something of the Lisp-way.
http://rapidshare.com/files/177299629/mine-text.lisp.html
http://rapidshare.com/files/177299946/defmodel.lisp.html
Thanks in advance for the attention.
Andrea Taverna
······@hotmail.it wrote:
> Hello everyone!
> I have spent the last months studying Lisp and playing with SLIME.
> I come up with the following two toy programs and I'd like to have
> them reviewed.
> In particular, I'd like to know whether the code is somewhat lispy, if
> it shows that I learnt something of the Lisp-way.
If "the lisp way" is related to "the one true way", then its something
you want to avoid. The right way to program is quickly, efficiently,
flexibly. Lisp is not a single-paradigm language. Programming is a
means, not an end.
Having finished these snippets, put them on the shelf and move on.
Write new code. If/when you are reusing these old snippets, you will
quickly learn whether they were written properly or not. Its the
whole hindsight thing.
In summation, your code was written in "the lisp way" IFF your code
complies with the CLHS.
Don't get caught navel gazing! (parens are so, sensual...)
But if you must,
http://norvig.com/luv-slides.ps
http://p-cos.net/lisp/guide.html
etc.
- Daniel
D Herring wrote:
> ······@hotmail.it wrote:
>
>> Hello everyone!
>> I have spent the last months studying Lisp and playing with SLIME.
>> I come up with the following two toy programs and I'd like to have
>> them reviewed.
>> In particular, I'd like to know whether the code is somewhat lispy, if
>> it shows that I learnt something of the Lisp-way.
>
>
> If "the lisp way" is related to "the one true way", then its something
> you want to avoid. The right way to program is quickly, efficiently,
> flexibly. Lisp is not a single-paradigm language. Programming is a
> means, not an end.
I have never heard The Lisp Way summarized so succinctly! Well, part of
it. You left out the rest, for which noobs posting their code normally
get, um, re-educated.
kenneth
On 27 Dic, 22:10, D Herring <········@at.tentpost.dot.com> wrote:
> If "the lisp way" is related to "the one true way", then its something
> you want to avoid.
The "lisp way" I mean is the way you do things with Lisp, nothing
more.
I think I should learn programming in Lisp 'cause it's "different"
from other languages, and to do so I shouldn't program in it with a "C-
ish accent", hence the question.
> In summation, yourcodewas written in "the lisp way" IFF yourcode
> complies with the CLHS.
I'd better write this down...
Andrea
······@hotmail.it wrote:
> On 27 Dic, 22:10, D Herring <········@at.tentpost.dot.com> wrote:
>> If "the lisp way" is related to "the one true way", then its something
>> you want to avoid.
> The "lisp way" I mean is the way you do things with Lisp, nothing
> more.
> I think I should learn programming in Lisp 'cause it's "different"
> from other languages, and to do so I shouldn't program in it with a "C-
> ish accent", hence the question.
Herr Herring's advice was sound in that part of the Lisp Way is the use
of multiple paradigmes, but went too far in suggesting that...
>
>> In summation, yourcodewas written in "the lisp way" IFF yourcode
>> complies with the CLHS.
...everything that compiles is The Lisp Way. See the free version of
Paul Graham's "On Lisp" for things that should be viewed as carrying a
tax, or post code here and you will certainly get pounded for anything
un-Lispy.
kt
······@hotmail.it writes:
> Thanks, forgive my ignorance...
>
> Here are the new links
> t http://paste.lisp.org/display/72709
> http://paste.lisp.org/display/72710
(defun read-char-token (stream length)
:documentation
"Reads a lexical token of given length from stream."
This doesn't document the function. Where did you get the notion that
:DOCUMENTATION was needed here?
C/USER[20]> (defun f ()
:documentation "Wrong"
'f)
F
C/USER[21]> (documentation 'f 'function)
NIL
C/USER[22]> (defun g ()
"Right"
'f)
G
C/USER[23]> (documentation 'g 'function)
"Right"
C/USER[24]>
(loop :for i :downfrom (1- length) :to 1
:do (unread-char (char token i) stream))
This doesn't work. Read again CLHS about UNREAD-CHAR.
You can unread at most one character.
(setf (gethash item table) `(,@(1+ freq)))
Why such an horror? Are you competing for some obfuscated lisp code contest?
(setf (gethash item table) '1)
There's no need to quote literal numbers, they're self evaluating, and
you can count on the compiler to avoid useless evaluations.
(defun set-token (length scanner line)
:documentation
"Used in create-word-token-reader to actually read the token.
If it is used to read the token, why isn't it called READ-TOKEN ?
(defun table-to-array (table)
:documentation "Convert a hash table to a 2d array. An element of the array is a 2-elements vector holding a key and the corresponding value."
(let ((array (make-sequence 'vector (hash-table-count table)))
(i 0))
(maphash
#'(lambda (k v)
(progn
(setf (aref array i) (vector k v))
(incf i)))
table)
array
))
This doesn't make a 2D array, but a vector of vectors.
(defun hashtable-to-array (hashtable)
"Convert a hash table to a 2d array.
Keys are stored in the column 0, and values in the column 1"
(let ((array (make-array (list (hash-table-count hashtable) 2)))
(i 0))
(maphash (lambda (k v)
(setf (aref array i 0) k
(aref array i 1) v)
(incf i))
hashtable)
array))
C/USER[35]> (HASHTABLE-TO-ARRAY #S(HASH-TABLE :test eql (one . 1) (two . 2) (three . 3)))
#2A((THREE 3) (TWO 2) (ONE 1))
In create-word-token-reader you have:
(let ((line-cursor 0)
(line nil))
(lambda (stream)
(let ((token "")
(first-word ""))
(when (not line)
(setf line (line-reader stream)))
What would happen if you did:
(let ((wtr (crate-word-token-reader ...)))
(wtr stream1)
(wtr stream2)
...)
If you need to keep state depending on the argument of the function,
then you shouldn't keep it in the closure, or you should enclose the
argument too.
In general there are too many comments. If your code is not clear
enough, make it clearer, don't write this kind of comments...
--
__Pascal Bourguignon__
In article
<····································@r37g2000prr.googlegroups.com>,
······@hotmail.it wrote:
> Thanks, forgive my ignorance...
>
> Here are the new links
> t http://paste.lisp.org/display/72709
> http://paste.lisp.org/display/72710
Some remarks...
Generally see Norvig/Pitman for good style:
http://lispm.dyndns.org/documentation/norvig-lisp-style.pdf
(defun read-char-token (stream length)
:documentation
"Reads a lexical token of given length from stream."
(let ((token (make-string length)))
(loop
; read length character or until the end of file
:for i :upfrom 0 :to (- length 1)
:for c := (read-char stream nil) :while c
:do (setf (char token i) c)
:finally
; if the token didn't complete before e.o.f., i.e. if the last
; character read is nil, return a void token
(if (not c)
(setf token "")
; put second second and following characters back into
; the stream for further reads
(loop :for i :downfrom (1- length) :to 1
:do (unread-char (char token i) stream))
)
)
token)
)
Documentation strings for functions work differently. No :documentation.
Write code more compact without single line parentheses...
(defun read-char-token (stream length)
"Reads a lexical token of given length from stream."
(let ((token (make-string length)))
(loop for i below length
for c = (read-char stream nil)
while c
do (setf (char token i) c)
finally (if (not c)
(setf token "")
(loop :for i :downfrom (1- length) :to 1
:do (unread-char (char token i) stream))))
token))
This does not work in general. You can't UNREAD-CHAR multiple times.
'It is an error to invoke unread-char twice consecutively on the same stream
without an intervening call to read-char (or some other input
operation which implicitly reads characters) on that stream.'
Why do you want to unread?
(defun read-single-char (stream)
:documentation "Reads a single character from stream."
(read-char-token stream 1))
Hmm, what does that do different from READ-CHAR?
(defun create-checked-reader (readf predicate)
:documentation "Creates a closure that reads from a given stream returning only the tokens that satisfy given predicate."
(lambda (stream)
; read until stream ends or text satisfy predicate
(loop
:for text := (funcall readf stream)
:while (and
(> (length text) 0)
(not (funcall predicate text)))
:finally
(return text)
)
))
It seems to read multiple times and return the first 'token' that satisfies a predicate.
(defun update-frequency (item table)
:documentation "Increase by one the frequency of the item <item> inside the frequency table <table>. New items, i.e. items not found in the table at call time,
have frequency 0."
(multiple-value-bind (freq present) (gethash item table)
(if present
(setf (gethash item table) `(,@(1+ freq)))
(setf (gethash item table) '1)
)
)
)
Replace above with (incf (gethash item table 0)) .
Anyway, `(,@(1+ freq)) makes no sense. It is just (1+ freq).
'1 is not needed. just 1 is fine. Numbers don't need to be quoted
because a number evaluates to the number itself.
(defun mine-text (file &optional (read-token-f #'read-single-char))
:documentation "Given a file and a reading function, returns a frequency table of the tokens inside the file.
Tokens are provided by the reading function.
The reading function must return nil when end of file is reached."
(let (
(table (make-hash-table :test #'equal))
)
(with-open-file (stream file :direction :input)
(loop
:for token :of-type string := (funcall read-token-f stream)
:while (> (length token) 0)
:do
(update-frequency token table)
))
table
))
(defun set-token (length scanner line)
:documentation
"Used in create-word-token-reader to actually read the token.
Takes the expected length of the token, a regular-expression scanner and the line from which the token is read.
If matching doesn't succeed, returns two empty string as the token and its first word."
(let (; word token
(token "")
; first word in token
(first-word ""))
; with length=1 first-word and token coincide
(if (= length 1)
(progn
(setf token (funcall scanner line))
(setf first-word token))
(multiple-value-bind (tok word) (funcall scanner line)
(setf token tok)
(if (> (length token) 0)
; scanner returns the first word inside a vector
(setf first-word (aref word 0))
; if no token was read, the scanner returns an empty vector
(setf first-word ""))
)
)
(values token first-word)
))
Better put comments on the same line after the construct: (do-something) ; I'm doing something
Provide only useful comments. The usual Lisp style is:
(let ((first-token-found nil)) ...
and not
(let ((ftf nil)) ... ; first token found
(defun table-to-array (table)
:documentation "Convert a hash table to a 2d array. An element of the array is a 2-elements vector holding a key and the corresponding value."
(let ((array (make-sequence 'vector (hash-table-count table)))
(i 0))
(maphash
#'(lambda (k v)
(progn
(setf (aref array i) (vector k v))
(incf i)))
table)
array
))
Within LAMBDA you don't need progn.
There is also no 2d array, despite your documentation string.
A 2d array is created for example by (make-array (list (hash-table-count table) 2))
Example:
(defun table-to-array (table)
"Convert a hash table to a 2d array."
(let ((array (make-array (list (hash-table-count table) 2)))
(i 0))
(maphash (lambda (k v)
(setf (aref array i 0) k
(aref array i 1) v)
(incf i))
table)
array))
If you write code:
don't
(f1
(f2
a1
a2
)
)
use
(f1 (f2 a1 a2))
or
(flongerlonger1 (f2longerlonger a1longer
a2longer))
Lisp allows you to write the expressions in any shape,
over multiple lines, with/without indentation, with
added whitespace, ... You can also quickly
select an expression with the editor. So, write
descriptive names (symbols) and compact code.
A name for a function or variable should be chosen
with care. You should be able to remember the name,
even guess the name. You can query Lisp about it.
(documentation 'read-all-tokens 'function)
(apropos "token")
(ed 'read-all-tokens)
(describe 'read-all-tokens)
--
http://lispm.dyndns.org/
Rainer Joswig wrote:
> In article
> <····································@r37g2000prr.googlegroups.com>,
> ······@hotmail.it wrote:
>
> > Thanks, forgive my ignorance...
> >
> > Here are the new links
> > t http://paste.lisp.org/display/72709
> > http://paste.lisp.org/display/72710
>
>
> Some remarks...
>
> Generally see Norvig/Pitman for good style:
>
> http://lispm.dyndns.org/documentation/norvig-lisp-style.pdf
>
>
> (defun read-char-token (stream length)
> :documentation
> "Reads a lexical token of given length from stream."
> (let ((token (make-string length)))
> (loop
> ; read length character or until the end of file
> :for i :upfrom 0 :to (- length 1)
> :for c := (read-char stream nil) :while c
> :do (setf (char token i) c)
> :finally
> ; if the token didn't complete before e.o.f., i.e. if the
> last ; character read is nil, return a void token
> (if (not c)
> (setf token "")
> ; put second second and following characters back into
> ; the stream for further reads
> (loop :for i :downfrom (1- length) :to 1
> :do (unread-char (char token i) stream))
> )
> )
> token)
> )
>
> Documentation strings for functions work differently. No
> :documentation. Write code more compact without single line
> parentheses...
>
> (defun read-char-token (stream length)
> "Reads a lexical token of given length from stream."
> (let ((token (make-string length)))
> (loop for i below length
> for c = (read-char stream nil)
> while c
> do (setf (char token i) c)
> finally (if (not c)
> (setf token "")
> (loop :for i :downfrom (1- length) :to 1
> :do (unread-char (char token i) stream))))
> token))
>
> This does not work in general. You can't UNREAD-CHAR multiple times.
> 'It is an error to invoke unread-char twice consecutively on the
> same stream without an intervening call to read-char (or some other
> input operation which implicitly reads characters) on that stream.'
> Why do you want to unread?
>
>
>
> (defun read-single-char (stream)
> :documentation "Reads a single character from stream."
> (read-char-token stream 1))
>
> Hmm, what does that do different from READ-CHAR?
>
> (defun create-checked-reader (readf predicate)
> :documentation "Creates a closure that reads from a given stream
> returning only the tokens that satisfy given predicate." (lambda
> (stream) ; read until stream ends or text satisfy predicate
> (loop
> :for text := (funcall readf stream)
> :while (and
> (> (length text) 0)
> (not (funcall predicate text)))
> :finally
> (return text)
> )
> ))
>
> It seems to read multiple times and return the first 'token' that
> satisfies a predicate.
>
> (defun update-frequency (item table)
> :documentation "Increase by one the frequency of the item <item>
> inside the frequency table <table>. New items, i.e. items not found
> in the table at call time, have frequency 0."
> (multiple-value-bind (freq present) (gethash item table) (if
> present (setf (gethash item table) `(,@(1+ freq)))
> (setf (gethash item table) '1)
> )
> )
> )
>
> Replace above with (incf (gethash item table 0)) .
> Anyway, `(,@(1+ freq)) makes no sense. It is just (1+ freq).
> '1 is not needed. just 1 is fine. Numbers don't need to be quoted
> because a number evaluates to the number itself.
>
>
> (defun mine-text (file &optional (read-token-f #'read-single-char))
> :documentation "Given a file and a reading function, returns a
> frequency table of the tokens inside the file. Tokens are provided
> by the reading function. The reading function must return nil
> when end of file is reached." (let (
> (table (make-hash-table :test #'equal))
> )
> (with-open-file (stream file :direction :input)
> (loop
> :for token :of-type string := (funcall read-token-f
> stream) :while (> (length token) 0)
> :do
> (update-frequency token table)
> ))
> table
> ))
>
>
> (defun set-token (length scanner line)
> :documentation
> "Used in create-word-token-reader to actually read the token.
> Takes the expected length of the token, a regular-expression
> scanner and the line from which the token is read. If matching
> doesn't succeed, returns two empty string as the token and its first
> word." (let (; word token (token "")
> ; first word in token
> (first-word ""))
> ; with length=1 first-word and token coincide
> (if (= length 1)
> (progn
> (setf token (funcall scanner line))
> (setf first-word token))
> (multiple-value-bind (tok word) (funcall scanner line)
> (setf token tok)
> (if (> (length token) 0)
> ; scanner returns the first word inside a vector
> (setf first-word (aref word 0))
> ; if no token was read, the scanner returns an empty
> vector (setf first-word ""))
> )
> )
> (values token first-word)
> ))
>
> Better put comments on the same line after the construct:
> (do-something) ; I'm doing something Provide only useful comments.
> The usual Lisp style is:
>
> (let ((first-token-found nil)) ...
>
> and not
>
> (let ((ftf nil)) ... ; first token found
>
>
>
> (defun table-to-array (table)
> :documentation "Convert a hash table to a 2d array. An element of
> the array is a 2-elements vector holding a key and the corresponding
> value." (let ((array (make-sequence 'vector (hash-table-count
> table))) (i 0)) (maphash
> #'(lambda (k v)
> (progn
> (setf (aref array i) (vector k v))
> (incf i)))
> table)
> array
> ))
>
> Within LAMBDA you don't need progn.
>
> There is also no 2d array, despite your documentation string.
> A 2d array is created for example by (make-array (list
> (hash-table-count table) 2))
>
> Example:
>
> (defun table-to-array (table)
> "Convert a hash table to a 2d array."
> (let ((array (make-array (list (hash-table-count table) 2)))
> (i 0))
> (maphash (lambda (k v)
> (setf (aref array i 0) k
> (aref array i 1) v)
> (incf i))
> table)
> array))
>
>
>
> If you write code:
>
> don't
>
> (f1
> (f2
> a1
> a2
> )
> )
>
> use
>
> (f1 (f2 a1 a2))
>
> or
>
> (flongerlonger1 (f2longerlonger a1longer
> a2longer))
>
> Lisp allows you to write the expressions in any shape,
> over multiple lines, with/without indentation, with
> added whitespace, ... You can also quickly
> select an expression with the editor. So, write
> descriptive names (symbols) and compact code.
>
> A name for a function or variable should be chosen
> with care. You should be able to remember the name,
> even guess the name. You can query Lisp about it.
>
> (documentation 'read-all-tokens 'function)
>
> (apropos "token")
>
> (ed 'read-all-tokens)
>
> (describe 'read-all-tokens)
Let's say that this is the data file:
param huge := 99. ;
param rtmin := 0.0 ;
param rtmax := 8.0 ;
param otmin := 0.0 ;
param otmax := 96.0 ;
param rmin := w01 0.00 w05 0.00 w96 0.00 ;
param rmax := w01 3.00 w05 2.00 w96 3.00 ;
param omin := w01 0.0 w05 0.0 w96 0.0 ;
param omax := w01 48.0 w05 0.0 w96 48.0 ;
param hd := w01 8.0 w05 8.0 w96 8.0 ;
param dp := w01 19.0 w05 19.0 w96 19.0 ;
Then we run this Ruby program:
require 'pp'
hash = {}
IO.read('data').strip.split( /\s*;\s*/ ).map{|line|
line.split( /[:=\s]+/ )}.each{|ary| hash[ary[1]] = ary[2..-1] }
pp hash
--- output ---
{"rmax"=>["w01", "3.00", "w05", "2.00", "w96", "3.00"],
"otmax"=>["96.0"],
"huge"=>["99."],
"dp"=>["w01", "19.0", "w05", "19.0", "w96", "19.0"],
"omax"=>["w01", "48.0", "w05", "0.0", "w96", "48.0"],
"rmin"=>["w01", "0.00", "w05", "0.00", "w96", "0.00"],
"otmin"=>["0.0"],
"rtmax"=>["8.0"],
"omin"=>["w01", "0.0", "w05", "0.0", "w96", "0.0"],
"hd"=>["w01", "8.0", "w05", "8.0", "w96", "8.0"],
"rtmin"=>["0.0"]}
William James schrieb:
> Let's say that this is the data file:
>
> param huge := 99. ;
>
> param rtmin := 0.0 ;
> param rtmax := 8.0 ;
>
> param otmin := 0.0 ;
> param otmax := 96.0 ;
>
>
> param rmin := w01 0.00 w05 0.00 w96 0.00 ;
> param rmax := w01 3.00 w05 2.00 w96 3.00 ;
>
> param omin := w01 0.0 w05 0.0 w96 0.0 ;
> param omax := w01 48.0 w05 0.0 w96 48.0 ;
>
> param hd := w01 8.0 w05 8.0 w96 8.0 ;
>
> param dp := w01 19.0 w05 19.0 w96 19.0 ;
>
>
> Then we run this Ruby program:
>
> require 'pp'
> hash = {}
> IO.read('data').strip.split( /\s*;\s*/ ).map{|line|
> line.split( /[:=\s]+/ )}.each{|ary| hash[ary[1]] = ary[2..-1] }
> pp hash
>
> --- output ---
> {"rmax"=>["w01", "3.00", "w05", "2.00", "w96", "3.00"],
> "otmax"=>["96.0"],
> "huge"=>["99."],
> "dp"=>["w01", "19.0", "w05", "19.0", "w96", "19.0"],
> "omax"=>["w01", "48.0", "w05", "0.0", "w96", "48.0"],
> "rmin"=>["w01", "0.00", "w05", "0.00", "w96", "0.00"],
> "otmin"=>["0.0"],
> "rtmax"=>["8.0"],
> "omin"=>["w01", "0.0", "w05", "0.0", "w96", "0.0"],
> "hd"=>["w01", "8.0", "w05", "8.0", "w96", "8.0"],
> "rtmin"=>["0.0"]}
Now let’s assume the data file looks like this:
{:huge 99.
:rtmin 0.0
:rtmax 8.0
:otmin 0.0
:otmax 96.0
:rmin ["w01" 0.0 "w05" 0.0 "w96" 0.0]
:rmax ["w01" 3.0 "w05" 2.0 "w96" 3.0]
:omin ["w01" 0.0 "w05" 0.0 "w96" 0.0]
:omax ["w01" 48.0 "w05" 0.0 "w96" 48.0]
:hd ["w01" 8.0 "w05" 8.0 "w96" 8.0]
:dp ["w01" 19.0 "w05" 19.0 "w96" 19.0]}
This could be data saved by Clojure programs.
Then your complicated 5 line Ruby program would become:
(read-string (slurp "data"))
Not for nothing people’s first thought is typically about the
programming language Comal, from the late 70ies, when they hear
about Ruby.
Saving such a db to a file could be done via duck-streams spit:
(spit "data" *db*)
where (def *db* {:huge 99., :rtmin 0.0, ...})
Now I would like to see your Ruby/Comal program that stores your output
back into the file "data".
Uhm no, now as I think about it... I don’t want to see that. Please not.
André
--
Lisp is not dead. It’s just the URL that has changed:
http://clojure.org/
Thanks again for the replies.
On 27 Dic, 21:32, Rainer Joswig <······@lisp.de> wrote:
[snip]
> This does not work in general. You can't UNREAD-CHAR multiple times.
> 'It is an error to invoke unread-char twice consecutively on the same stream
> without an intervening call to read-char (or some other input
> operation which implicitly reads characters) on that stream.'
Uh, I missed that.
> Why do you want to unread?
Because I need the characters of the old token to compose the new one.
Uh, never mind. I'll make the function returning a closure that stores
characters in a vector/list, so that I don't need to put the
characters back into the stream.
······@hotmail.it writes:
> Thanks again for the replies.
>
> On 27 Dic, 21:32, Rainer Joswig <······@lisp.de> wrote:
> [snip]
>> This does not work in general. You can't UNREAD-CHAR multiple times.
>> �'It is an error to invoke unread-char twice consecutively on the same stream
>> �without an intervening call to read-char (or some other input
>> �operation which implicitly reads characters) on that stream.'
> Uh, I missed that.
>> Why do you want to unread?
> Because I need the characters of the old token to compose the new one.
> Uh, never mind. I'll make the function returning a closure that stores
> characters in a vector/list, so that I don't need to put the
> characters back into the stream.
Good! :-)
--
__Pascal Bourguignon__
······@hotmail.it writes:
> Thanks, forgive my ignorance...
>
> Here are the new links
> t http://paste.lisp.org/display/72709
> http://paste.lisp.org/display/72710
; given a string, returns the corresponding keyword symbol
(defun make-keyword (string)
(intern (string-upcase string) :keyword))
1- documentation strings are to be inserted in the defun form, after the
parameter list, not as comment before the function.
2- string-upcase takes a string designator, so your function accepts it
too. If you don't like it, you could use CHECK-TYPE to ensure only
strings are accepted.
3- do not write misleading comments or documentation strings. Better
not write anything than writting something wrong. Speakers of the
language will understand the native code without needed for a
mistranslated English version.
4- choose a function name matching the function meaning.
(defun make-uppercased-keyword (string-designator)
"Given a string-designator, returns the keyword symbol with the name upper cased."
(intern (string-upcase string-designator) :keyword))
(defun make-keyword (string-designator)
"Given a string-designator, returns the keyword symbol with that string as name."
(intern (string string-designator) :keyword))
(mapcar (lambda (f) (mapcar (lambda (s) (funcall f s))
'(|abc| abc "abc" "ABC" #\a #\A)))
(list (function MAKE-UPPERCASED-KEYWORD) (function MAKE-KEYWORD)))
--> ((:ABC :ABC :ABC :ABC :A :A)
(:|abc| :ABC :|abc| :ABC :|a| :A))
Notice how make-keyword is more powerful than make-uppercased-keyword
(the image of make-keyword contains the image of
make-uppercased-keyword). This should be a hint that unless you have
particular circumstances, you should rather use the later...
#| defines a new symbol from an old one.
Name is constructed concatenating prefix, sym and suffix.
Sym is the old symbol. |#
(defun conc-sym (prefix sym suffix)
(intern (concatenate 'string prefix (string sym) suffix )))
1- see above
2- you could easily generalize this function for any number of argument,
of any type.
3- see above
(defun conc-sym (&rest arguments)
"interns a new symbol whose name is the concatenation of the
princ representation of each argument."
(intern (format nil "~{~A~}" arguments)))
(conc-sym 'abc -42 '- "DEF")
--> ABC-42-DEF ;
NIL
#|
Given a pair ("A" A) where A is a symbolic name and "A" the textual representation for a parameter A,
returns the corresponding slot declaration for the model class
|#
(defun parameter->slot (par-name)
(let ((name (elt par-name 1)))
`(,name :initarg ,(make-keyword (string name)))
))
1- see above
2- NEVER WRITE COMMENTS that are wrong. Above you wrote:
Let's assume that symbolic names are those used in the code. Those
used in the files are "textual-names". Then a parameter can be
coded as a pair (N T) where N is the symbolic name and T the
textual one, and the model as an alist ((N T)...)
here you're writting ("A" A), so (T N). What is it?
(Textual Symbolic) or (Symbolic Textual)?
It's safer just to not write any "comment", and to use
self-documenting names in the code.
3- given that MAKE-KEYWORD takes a string designator, and that it's
probably better to return a slot whose init-arg is of the same case
as the name, just writting:
(defun parameter->slot (par-name)
"Given a pair of textual and symbolic names,
returns the corresponding slot declaration for the model class,
made using the symbolic name."
(destructuring-bind (textual-name symbolic-name) par-name
`(,symbolic-name :initarg ,(make-keyword symbolic-name))))
(mapcar 'parameter->slot '(("ABC" ABC) ("abc" |abc|)))
--> ((ABC :INITARG :ABC) (|abc| :INITARG :|abc|))
(defmacro defmodel (name parameters &key ((:extends supermodels)))
(let (; builds supermodels' data classes from supermodels list
(supermodels-data
(mapcar #'(lambda (name) (conc-sym nil name "-DATA")) supermodels))
; builds model data class name
(name-data (conc-sym nil name "-DATA"))
; builds global parse-table variable name
(partable-var (conc-sym "*" name "-PARTABLE*"))
; builds global parse-table variables names from supermodels list
(super-partable-vars (let ((var-names (mapcar #'(lambda (name) (conc-sym "*" name "-PARAMS*")) supermodels)))
(mapcar #'eval var-names)))
; final value of parse-table for the model
(partable-var-value))
(progn
(setf partable-var-value (append (reduce #'append super-partable-vars) parameters ) )
`(progn
,`(defvar ,partable-var ',partable-var-value)
,`(defclass ,name-data ,supermodels-data
,(append (map 'list #'parameter->slot parameters) (list (partable->slot partable-var) )))
))
))
1- don't write useless comments.
2- better use a symbol as a string designator to build symbols than
strings, since then the case will be automatically set according to the
current readtable case. "-DATA" ==> '-data or :-data
3- learn about read-time, macro-expansion time, compilation time, load
time, and execution time, and about effects occuring at each time for
the various class of operators.
The problem you have here, is that DEFVAR ensures that the value of
the variable is set as a run-time effect, not as a compilation time
effect. With most implementations, macroexpansion time will occur
within compilation time, and therefore evaluation of y our variable
name will occur at a time when these variables have no value.
This is just an example of why you shouldn't use EVAL until you reach
black belt 2nd dan in CL.
4- Read clhs about DEFVAR and notice the differences with DEFPARAMETER.
5- instead of defining variables, for the partables, I'd define a method
specialized on the class names. We could avoid using the class
allocation slot, since we'd need to create an instance to be able to
access this slot.
6- is there a good reason not to put the models in the same namespace as
the classes? Suffixing a -data doesn't really separates the
namespaces, and it would give clearer code if you let the user name
his things himself.
(defgeneric model-parameter-table (model)
(:documentation "Return a list of (textual-name symbolic-name) couples
defining all the parameters of the model, including the inherited ones."))
(defmacro defmodel (name parameters &key ((:extends supermodels)))
`(progn
(defmethod model-parameter-table ((model (eql ',name)))
(append ',parameters
,@(mapcar (lambda (supermodel) `(model-parameter-table ',supermodel))
supermodels)))
(defclass ,name ,supermodels
(,@(mapcar (function parameter->slot) parameters)))))
C/USER[63]> (defmodel coche (("wheels" wheels) ("motor" motor)) :extends (vehicule))
#1=#<STANDARD-CLASS COCHE :INCOMPLETE>
C/USER[64]> (defmodel vehicule (("weight" weight)))
#1=#<STANDARD-CLASS VEHICULE>
C/USER[65]> (inspect (make-instance 'coche))
#<COMMON-LISP-USER::COCHE #x1A233339>: standard object
type: COMMON-LISP-USER::COCHE
0 [WEIGHT]: |#<unbound>|
1 [WHEELS]: |#<unbound>|
2 [MOTOR]: |#<unbound>|
INSPECT-- type :h for help; :q to return to the REPL ---> :q
C/USER[66]> (model-parameter-table 'coche)
(("wheels" WHEELS) ("motor" MOTOR) ("weight" WEIGHT))
C/USER[67]> (macroexpand '(defmodel coche (("wheels" wheels) ("motor" motor)) :extends (vehicule)))
(PROGN (DEFMETHOD MODEL-PARAMETER-TABLE ((MODEL (EQL 'COCHE)))
(APPEND '(("wheels" WHEELS) ("motor" MOTOR)) (MODEL-PARAMETER-TABLE 'VEHICULE)))
(DEFCLASS COCHE (VEHICULE) ((WHEELS :INITARG :WHEELS) (MOTOR :INITARG :MOTOR)))) ;
T
Notice how setting things up to be computed at run-time allows for the
defining (and redefining) of model classes in any order.
--
__Pascal Bourguignon__
On Dec 28, 2:00 am, Kenny <·········@gmail.com> wrote:
> Pascal J. Bourguignon wrote:
> > 2- NEVER WRITE COMMENTS that are wrong.
>
> That sentence should be three words shorter. Which left as an exercise.
Oh dat's eazy.
WRITE COMMENTS wrong.
Job security.
--
Geoff
Geoffrey Summerhayes wrote:
> On Dec 28, 2:00 am, Kenny <·········@gmail.com> wrote:
>
>>Pascal J. Bourguignon wrote:
>>
>>>2- NEVER WRITE COMMENTS that are wrong.
>>
>>That sentence should be three words shorter. Which left as an exercise.
>
>
> Oh dat's eazy.
>
> WRITE COMMENTS wrong.
Pretty close. Code is unambiguous. Natural language in the hands of a
master is thoroughly ambiguous. In the hands of a computer
programmer...oh, my.
Which would normally bring us to literate programming but I need to cut
Knuth a break.
kenneth
······@hotmail.it writes:
> Hello everyone!
> I have spent the last months studying Lisp and playing with SLIME.
> I come up with the following two toy programs and I'd like to have
> them reviewed.
> In particular, I'd like to know whether the code is somewhat lispy, if
> it shows that I learnt something of the Lisp-way.
>
> http://rapidshare.com/files/177299629/mine-text.lisp.html
> http://rapidshare.com/files/177299946/defmodel.lisp.html
There's no lisp code there, only some advertising for web hosting or
something...
--
__Pascal Bourguignon__
···@informatimago.com (Pascal J. Bourguignon) writes:
>> http://rapidshare.com/files/177299629/mine-text.lisp.html
>> http://rapidshare.com/files/177299946/defmodel.lisp.html
>
> There's no lisp code there, only some advertising for web hosting or
> something...
If you search a button with text like 'Download' and klick it, you get
functions like:
(defun update-frequency (item table)
:documentation "Increase by one the frequency of the item <item> inside the frequency table <table>. New items, i.e. items not found in the table at call time,
have frequency 0."
(multiple-value-bind (freq present) (gethash item table)
(if present
(setf (gethash item table) `(,@(1+ freq)))
(setf (gethash item table) '1)
)
)
)
Positive: documentation
Negative: c-style indentation, never use e new line for a closing
parentheses.
This
`(,@(1+ freq))
looks complicated to me, wouldn't do (1+ freq) do the same?
Jens