Sudoku table generator error, lisp

Question

I have a problem with some part of my lisp code. This is a sudoku table generator. It works fine to this part:

(loop for e in entries do (if (and (not (member e sub)) (not (member e col))) (progn (setq choices (nconc choices (list e))) (print choices))) (if (= (length choices) 1) (setq pick (car choices)) (if (not (= (length choices) 0)) (setq pick (nth (random (+ 0 (length choices))) choices)))) 

Basically, I am in row x and column y, and I need to insert an element. I look at the submatrix for this element and column, and I select a number that does not appear in any of the above, and put it there. This is the "pick" variable. The problem is that sometimes the "select" variable gets the NIL value, although in the record loops it has the correct value. When it gets NIL, the selection value remains the same as in the last loop (I iterate over the columns and rows above this snippet), resulting in my resulting table having invalid output (e.g. double values ​​in a row). How can I track where the selection variable changes? I work with him only in this fragment, and I do not understand why he suddenly changes to zero.

For example, I usually have:

• in recording cycles: selection (5)
• Exiting the cycle: selection (5)
• in recording cycles: selection (6 7)
• Exit the cycle: select (6 7), and after that:
• in write loops: select nil.

Thanks.

Answer 1

First, some reformatting:

 (loop for e in entries do (if (and (not (member e sub)) (not (member e col))) (progn (setq choices (nconc choices (list e))) (print choices))) (if (= (length choices) 1) (setq pick (car choices)) (if (not (= (length choices) 0)) (setq pick (nth (random (+ 0 (length choices))) choices)))) 

Then, if you don't need an alternative if clause, but want progn , you can use when :

 (loop for e in entries do (when (and (not (member e sub)) (not (member e col))) (setq choices (nconc choices (list e))) (print choices)) (if (= (length choices) 1) (setq pick (car choices)) (if (not (= (length choices) 0)) (setq pick (nth (random (+ 0 (length choices))) choices)))) 

The last two if clauses are mutually exclusive, so cond or case is suitable (I will use cond ):

 (loop for e in entries do (when (and (not (member e sub)) (not (member e col))) (setq choices (nconc choices (list e))) (print choices)) (cond ((= (length choices) 1) (setq pick (car choices))) ((not (= (length choices) 0)) (setq pick (nth (random (+ 0 (length choices))) choices)))) 

There is a zerop predicate:

 (loop for e in entries do (when (and (not (member e sub)) (not (member e col))) (setq choices (nconc choices (list e))) (print choices)) (cond ((= (length choices) 1) (setq pick (car choices))) ((not (zerop (length choices))) (setq pick (nth (random (+ 0 (length choices))) choices)))) 

I do not see that 0 should be added to some value:

 (loop for e in entries do (when (and (not (member e sub)) (not (member e col))) (setq choices (nconc choices (list e))) (print choices)) (cond ((= (length choices) 1) (setq pick (car choices))) ((not (zerop (length choices))) (setq pick (nth (random (length choices)) choices)))) 

If you are not sure if the pick parameter is set to a reasonable default, you should have a default case (this may be one of your problems):

 (loop for e in entries do (when (and (not (member e sub)) (not (member e col))) (setq choices (nconc choices (list e))) (print choices)) (cond ((= (length choices) 1) (setq pick (car choices))) ((not (zerop (length choices))) (setq pick (nth (random (length choices)) choices))) (t (setq pick nil)) 

Instead of using setq and nconc you can use push (this puts a new item at the top of the list, but since you randomly select randomly, this should not be a concern):

 (loop for e in entries do (when (and (not (member e sub)) (not (member e col))) (push e choices) (print choices)) (cond ((= (length choices) 1) (setq pick (car choices))) ((not (zerop (length choices))) (setq pick (nth (random (length choices)) choices))) (t (setq pick nil)) 

I suspect that at the beginning of this fragment choices should be () , that you do not need choices after this fragment, and the choices print is for debugging only, so you could do it differently using remove-if and changing the condition:

 (let ((choices (remove-if (lambda (e) (or (member e sub) (member e col))) entries))) (print choices) (cond ((= (length choices) 1) (setq pick (car choices))) ((not (zerop (length choices))) (setq pick (nth (random (length choices)) choices))) (t (setq pick nil))) 

If choices now prints as () , that means there is no choice, so you will have to digress (or whatever your algorithm does when the deadlock is reached).

Finally, since (length choices) can only be a non-negative integer, you can use case instead of cond if you check cases in a different order:

 (let ((choices (remove-if (lambda (e) (or (member e sub) (member e col))) entries))) (print choices) (case (length choices) (0 (setq pick nil)) (1 (setq pick (car choices))) (otherwise (setq pick (nth (random (length choices)) choices))))) 

Update on request.

As Rainer points out, this is basically the body of the pick function, so we can get rid of all the free variables. Alternatively, instead of car you can use (for lists) the more descriptive name first :

 (defun pick (entries sub col) (let ((choices (remove-if (lambda (e) (or (member e sub) (member e col))) entries))) (print choices) (case (length choices) (0 nil) (1 (first choices)) (otherwise (nth (random (length choices)) choices))))) 

This function will be defined elsewhere, and at the fragment location it will be called like this:

 (pick entries sub col) 

In order not to double (length choices) , we can put this in let (which needs to become let* for consistent evaluation):

 (defun pick (entries sub col) (let* ((choices (remove-if (lambda (e) (or (member e sub) (member e col))) entries)) (choices-length (length choices))) (print choices) (case choices-length (0 nil) (1 (first choices)) (otherwise (nth (random choices-length) choices))))) 

The last step (truly optional, but you might find that you have more sequences that reduce your options, like row ) will be a small generalization:

 (defun pick (entries &rest exclusion-sequences) (let* ((choices (remove-if (lambda (e) (some #'identity (mapcar (lambda (seq) (member e seq)) exclusion-sequences))) entries)) (choices-length (length choices))) (print choices) (case choices-length (0 nil) (1 (first choices)) (otherwise (nth (random choices-length) choices))))) 

The call to this function has the same form, but now you can use any number of exception sequences:

 (pick entries col sub row ver ima fou) 

Answer 2

A potential source of problems is NCONC.

• nconc destroys the first list. If this is not desired, use APPEND instead.

The second source of the NCONC problem is the use of literal lists.

Example:

 (defun foo (bar) (let ((l '(1 2 3))) ...)) 

Here (1 2 3) is a literal list. The effects of destructively changing such an undefined list in Common Lisp. Therefore, this should be avoided. What to do instead?

• minus list: (list 1 2 3)
• copy list of literals: (copy-list l)
• use non-destructive operations (APPEND instead of NCONC, ...)

Answer 3

My Lisp is pretty rusty, but I don’t see any digressions there ... and I think you can’t just start entering numbers randomly and expect them to make the right sudoku game.

It seems that the list is zero because there are no possible options and therefore is not created. You have to handle this.

Answer 4

This is not the correct answer, but I corrected the indentation to make the code more understandable for myself and other responders:

 (loop for e in entries do (if (and (not (member e sub)) (not (member e col))) (progn (setq choices (nconc choices (list e))) (print choices) )) (if (= (length choices) 1) (setq pick (car choices)) (if (not (=(length choices) 0)) (setq pick (nth (random (+ 0 (length choices))) choices)))) 

Questions:

• Are entries a list of lists? Does each list represent a string?
• What are the values ​​of "sub" and "col" for?