Why doesn't "cond" or "if" work inside when

I have written a "cond" inside "when" which is inside "loop". But it doesn't get evaluated. I have also tried "if". They independently exe in the interpreter but when written inside the loop Error: In - of (NIL 1) arguments should be of type NUMBER. What am I doing wrong ? Is there another way to check a condition inside loop?

(loop for i in kb
when (and (equal (length sos) 2)
         (member (second sos) pred1)
		(not (equal(nth notpos pred1) 'not)))
do (setq output (remove (second sos) pred1))
and do (setq output (rest output))
and do (setq soslist (append (list output) soslist))
	and do (setq sos output)
	and [b]do (print sos)[/b] ;prints until length becomes 1
	and do(setq clauses (rest clauses))
	and do(setq pred1 (first (rest clauses)))
	and do [b][color=#FF0000](cond ((= (length sos) 1)  (setq notpos (- (position (first sos) pred1) 1)))
			(t (setq notpos (- (position (second sos) pred1) 1))))[/color]
[/b]

else when (and (equal (length sos) 1)
         (member (first sos) pred1)
		 (equal(nth notpos pred1) 'not)) 
do (setq output (remove-at pred1 (+ notpos 1)))
and do (setq output (remove (first sos) output))
and do (setq output (rest output))
and do (setq soslist (append (list output) soslist))
	and do (setq sos output)
	and do(setq clauses (rest clauses))
	and do(setq pred1 (first (rest clauses)))
	and do(setq notpos (- (position (second sos) pred1) 1)))

more details: sos for 1st few times is a length 2 list, then it turns to a length 1 list. Then the cond doesnt get exe and I get an error. Could someone help me?

What is it supposed to do? Damned that is a complicated LOOP, can you simplify/break it up? Some notes:

• SETQ on a nonexistant variable makes a global(or is it special?) variable, LET, function argument variables, DESTRUCTURING-BIND, etcetera all make local variables, and DEFVAR, DEFPARAMETER make special variables.(will behave globally unless someone redefines them as opposed to setting them.)
• LOOP is a 'weird' macro.
• If you want things to be EQUAL as in numbers, = is better, for symbols, EQL, for general lists and stuff EQUALP(though i am not sure of the difference between it and EQUAL), if you want the adresses the same, EQ. (For that one, identical stuff might not have identical adresses.
• This board \[b\] in \[code\] tags don't work.

I tried to figure out what you were trying to do from the code, but i failed.. sorry.(I don't know LOOP particularly well either)

With so many clauses, this code is hard to read. Bugs often stand out when code is easy to read.

I would recommend collapsing all the "and do"s into one "do (progn ...)". For even better readability, how about the following?

(dolist (i kb)
  (cond
    ((and (equal (length sos) 2)
          (member (second sos) pred1)
       ...)
      ;; first bunch of do's
    )
    ((and (equal (length sos) 1)
          (member (first sos) pred1)
       ...)
      ;; second bunch of do's
    )))

@Jasper

my bad. I guessed it's not understandable.
I'm comparing if a list has a "not" so I used equal. I know its complicated but the error is at the point I tried to bold.
It is not executing the cond statement that is embedded inside those when and loop.

@nuntius

Thanks, ya I collapsed those do's. I'm completely new to LISP so I donno the alternatives.

The problem is not with the "cond".
The sos has an element that is not in the pred1 and so the error is coming.
But this is not the functionality I want. What I want to do is

for each sos (could be 1 or 2 elements like (g) or (not g))
I want to check if "g" is in the clauses(reverse kb) list.
If there is a match put it in the "pred" list and do one of the "and's"
How do we write someth like "for each sos loop..." in LISP?

This is the kb list (KB '((or c d) (or a b) (or not d b) (or not c e) (or not c a) (or not e f) (or not b f) (or not f g) (not g))).

Jasper wrote:

• If you want things to be EQUAL as in numbers, = is better, for symbols, EQL, for general lists and stuff EQUALP(though i am not sure of the difference between it and EQUAL)

Equalp works in two structures with equivalent fields while equal doesn't, possibly among other differences which I don't recall.

nuntius wrote:With so many clauses, this code is hard to read. Bugs often stand out when code is easy to read.

I would recommend collapsing all the "and do"s into one "do (progn ...)".

Not necessary: you can have more than one form in a do clause, without a progn or all the "and do" nonsense. But DOLIST is a better choice.

I feel the slightly verbose

(loop ...
  do (progn
       (thing1)
       (thing2)))

is clearer than the equivalent

(loop ...
  do (thing1)
     (thing2))

I don't think this is best done with LOOP.. How about:

(defun clause-says (about clause &key (test #'eql))
  (flet ((says (c) ;just a short for clause-says..
	   (clause-says about c :test test)))
    (typecase clause
      (symbol
       (funcall test about clause))
      ((cons (eql or) list)
       (find-if #'says (cdr clause)))
      ((cons (eql and) list)
       (not (find-if-not #'says (cdr clause))))
      ((cons (eql not) list) ;Decided to make (not ..) to be (not (and ...))
       (find-if-not  #'says (cdr clause))))))

It expects a little different input than you suggested though; everything is the same; toplevel must be a clause, and NOT is not read as a symbol, but also as an expression. IMO it makes more sense that way. I also added a way to change the test.

Edit: if i am understanding you correctly, this input should be somewhat equivalent

'(and (or c d) (or a b) (or (not d) (not b)) (or (not c) (not e)) (or (not c) (not a)) (or (not e) (not f)) (or (not b) (not f)) (or (not f) (not g)) (not g)))

Ftr, it can be simplified to (or (and (not ..) ..) .. etc..) I mean with just one OR, and max nesting of 3. I know how to do this. (It is not that hard.) Was thinking about writing a little blogpost about simplification. (As it is needed for this.)