problems with lisp and do construct with itterating

Question

Firstly, I am having this problem. The code I generated isn't iterating thru each word, but instead thru the entire passed argument. I am using a do loop to pass information into a hash table.

(defun set_isa (partOfSpeech &rest words)

(do ((wordVar words))
((null wordVar) nil) 

(putp partOfSpeech word-dict wordVar)
(setf wordVar (cdr wordVar))))

With this I am getting this as a result using trace

(set_isa 'Verb 'Talk 'Run 'jump )

1. Trace: (SET_ISA 'VERB 'TALK 'RUN 'JUMP)
1. Trace: SET_ISA ==> NIL
NIL

And when I call the hashtable, it only adds the last passed argument

#S(HASH-TABLE :TEST FASTHASH-EQL (VERB . (JUMP)))

Answer 1

So, the way to understand what is going on here is to annotate your code so it tells you what it's doing. This may seem an antique way of debugging, but in a dynamic, conversational language like CL it's a really good approach. Here is a version of your function using more conventional names for things, and also using conventional indetnation, together with stubs for your missing code to make it all runnable.

(defvar *word-dict* nil)

(defun set-isa (part-of-speech &rest words)
  (do ((wtail words))
      ((null wtail) nil) 
    (putp part-of-speech *word-dict* wtail)
    (setf wtail (cdr wtail))))

(defun putp (part-of-speech dict thing)
  (format *debug-io* "~&putp: ~A ~A ~A~%" part-of-speech dict thing))

So this is now runnable, and putp will print what it gets as arguments.

 > (set-isa 'verb 'talk 'run 'jump )
putp: verb nil (talk run jump)
putp: verb nil (run jump)
putp: verb nil (jump)
nil

So even without having the code which is actually buggy here, which is almost certainly putp, you can work out what the problem is: putp replaces whatever value is stored in the hash table with its argument. So the only value that ends up in the table is the last one. So we need to fix that, which I'll do later.

But in fact that's not the only problem.

First of all you're using do in a very odd way. The syntax of do explicitly allows initialisation and stepping forms, so you should actually use the stepping form rather than doing it in the body.

(defun set-isa (part-of-speech &rest words)
  (do ((wtail words (rest wtail)))
      ((null wtail) nil) 
    (putp part-of-speech *word-dict* wtail)))

Secondly you're calling putp on all the tails of your list: you probably want to be calling it on just the individual words. You could do this by simply passing it the car of each tail, but as Martin Buchmann points out in the other answer you might instead look for a construct in the language which iterates over the elements of a list. And there are plenty of these, and dolist is one of them:

(defun set-isa (part-of-speech &rest words)
  (dolist (word words)
    (putp part-of-speech *word-dict* word)))

And now

(set-isa 'verb 'talk 'run 'jump )
putp: verb nil talk
putp: verb nil run
putp: verb nil jump
nil

Note the way putp is called is not compatible with the previous version: it's now called on the words, not the tails of the list.

So finally, let's write a version of putp that works. I'll first write a very naive version:

(defvar *word-dict* (make-hash-table))

(defun putp (part-of-speech dict thing)
  (let ((entries (gethash part-of-speech dict '())))
    (setf (gethash part-of-speech dict) (cons thing entries))))

And this works, but not in a very good way:

> (gethash 'verb *word-dict* '())
nil
nil

> (set-isa 'verb 'talk 'run 'jump )
nil

> (gethash 'verb *word-dict* '())
(jump run talk)
t

> (set-isa 'verb 'talk 'run 'jump )
nil

> (gethash 'verb *word-dict* '())
(jump run talk jump run talk)
t

So that's OK, so long as you only run it once. Well, we can do better than this:

• we can use a more idiomatic way of pushing new things onto the list stored in the hashtable;
• we can avoid the duplicate entry thing while also being more idiomatic.

Like this:

(defun putp (part-of-speech dict thing)
  (pushnew thing (gethash part-of-speech dict)))

So, now:

 > (gethash 'verb *word-dict* '())
nil
nil

> (set-isa 'verb 'talk 'run 'jump )
nil

> (gethash 'verb *word-dict* '())
(jump run talk)
t

> (set-isa 'verb 'talk 'run 'jump )
nil

> (gethash 'verb *word-dict* '())
(jump run talk)

This is much better. You can look up push, and pushnew to see what they do.

Answer 2

Welcome to SO. I see a few issues with our question and hope I can give you some hints.

Indentation

Please indent your code properly. This will lead to more readable code and will increase the probability that others can help you.

(defun set_isa (partOfSpeech &rest words)
  "Put an understandable docstring here!"
  (do ((wordVar words))
      ((null wordVar) nil) 
    (putp partOfSpeech word-dict wordVar)
    (setf wordVar (cdr wordVar))))

You will find more advice on style here. See also the Info tab for a collection of references.

Runable examples

In our function putp isn't defined. So I cannot run your code and see what error to get exactly, etc. Please give always complete examples and a clear description of what you expect and what you get. The part with the hash-table isn't clear at all. Where does it come from? How is it used in our code?

do loops

Check a beginner reference for the correct syntax of do and its relatives. If you want to iterate through a list using do try dolist.

(dolist (item '(a b c d))
  (print item))

Using do you could achieve the same with this construct:

(do ((items '(a b c d) (rest items)))
    ((null items))
  (print (first items)))

I might seem tricky to get the parenthesis right in the first place but if you've got the logic behind it right it will become easier. You don't need the setf part as do will take care of it.