Program problem

Program problem

[Lewis Creary via Users list for the GNU Emacs text editor]

(It has been suggested to me by someone on the above mailing list thatthe following content would be appropriate for this list.)
My purpose here is to discuss a bug I've discovered in an emacs lispfunction, fill-rows, that I've written (shown below).  The main ideaof this function is to help solvers of Sudoku puzzles who want to keeptrack of the numbers they've already entered into a puzzle.  This willhelp them to backtrack when they change some of those numbers.  It'simportant to understand the documentation string of this function(included in the function definition) before reading the code.
The use case of interest involves this function call:   (fill-rows '(1 3 5)                  '("123456789" "987654321" "123498765")                   '((0 0 0 0 0 0 0 0 0)                    (0 0 0 0 0 0 0 0 0)                    (0 0 0 0 0 0 0 0 0)                    (0 0 0 0 0 0 0 0 0)                    (0 0 0 0 0 0 0 0 0)                    (0 0 0 0 0 0 0 0 0)                    (0 0 0 0 0 0 0 0 0)                    (0 0 0 0 0 0 0 0 0)                    (0 0 0 0 0 0 0 0 0)) )          
The desired value to be computed by this function call is:              '((1 2 3 4 5 6 7 8 9)                 (0 0 0 0 0 0 0 0 0)                (9 8 7 6 5 4 3 2 1)                (0 0 0 0 0 0 0 0 0)                (1 2 3 4 9 8 7 6 5)                (0 0 0 0 0 0 0 0 0)                (0 0 0 0 0 0 0 0 0)                (0 0 0 0 0 0 0 0 0)                (0 0 0 0 0 0 0 0 0))
The value (incorrectly) computed (this is the bug mentioned above) is:              '((0 0 0 0 0 0 0 0 0)                (9 0 0 0 0 0 0 0 0)                (0 0 0 0 0 0 0 0 0)                (0 0 0 0 0 0 0 0 0)                (0 0 0 0 0 0 0 0 0)                (0 0 0 0 0 0 0 0 0)                (0 0 0 0 0 0 0 0 0)                (0 0 0 0 0 0 0 0 0)                (0 0 0 0 0 0 0 0 0))
Here, then, is the function definition:
(defun fill-rows (row-nums change-strings pzl-form)  "This fn takes as arguments a list of row indices, a list of an equal   number, s, of 9-digit strings, and a puzzle form (i.e, an sexp of the form:                  ((n n n n n n n n n)                   (n n n n n n n n n)                   (n n n n n n n n n)                   (n n n n n n n n n)                   (n n n n n n n n n)                   (n n n n n n n n n)                   (n n n n n n n n n)                   (n n n n n n n n n)                   (n n n n n n n n n)), where n is a number between 0 and 9).   The fn returns as value t he result of filling in row m of the argument   puzzle form with numbers specified by the mth 9-digit string."  (let* ((row-index 0)           (row-num-index 0)           (row-str (nth row-index change-strings))           (row-num (nth row-num-index row-nums))           (change-nums nil)           (str-pos 0) )      (while (and (< row-index (length row-nums)) (< str-pos 9))             ;; (comment) cycle through the change-strings          (while (< str-pos 9)                 ;; cycle through row-str, contributing to change-list                   (setq change-nums (append change-nums                                                   (list (string-to-number                                                      (substring row-str str-pos (1+ str-pos)) )) )                        row-str (nth row-index change-strings) )              (setcar (car (nthcdr row-num pzl-form))                          (nth str-pos change-nums) )          (cl-incf str-pos) )       (cl-incf row-index)       (cl-incf row-num-index) )   pzl-form ) )
----------------------------------------------------------------------------------------------------
I'll very much appreciate receiving any proposed bugfixes or other  suggestions you may have.
Thanks.
Lewis Creary

Re: Program problem

[Emanuel Berg]

Lewis Creary via Users list for the GNU Emacs text editor wrote:

> It has been suggested to me by someone on the above mailing
> list thatthe following content would be appropriate for
> this list.

It is fine but that file got messy, do you have it uploaded
somewhere?

-- 
underground experts united
https://dataswamp.org/~incal

Re: Program problem

[Manuel Giraud via Users list for the GNU Emacs text editor]

Lewis Creary via Users list for the GNU Emacs text editor
<help-gnu-emacs@gnu.org> writes:

> (It has been suggested to me by someone on the above mailing list
> thatthe following content would be appropriate for this list.)

Hi,

First things first, if you want some help you should make your code
readable to others.  It may be your mail client that does that but your
text code is completely messed up:

[...]

> (defun fill-rows (row-nums change-strings pzl-form)  "This fn takes as
> arguments a list of row indices, a list of an equal   number, s, of
> 9-digit strings, and a puzzle form (i.e, an sexp of the form:         
>         ((n n n n n n n n n)                   (n n n n n n n n n)   
>                (n n n n n n n n n)                   (n n n n n n n n
> n)                   (n n n n n n n n n)                   (n n n n n
> n n n n)                   (n n n n n n n n n)                   (n n
> n n n n n n n)                   (n n n n n n n n n)), where n is a
> number between 0 and 9).   The fn returns as value t he result of
> filling in row m of the argument   puzzle form with numbers specified
> by the mth 9-digit string."  (let* ((row-index 0)         
>  (row-num-index 0)           (row-str (nth row-index change-strings)) 
>          (row-num (nth row-num-index row-nums))           (change-nums
> nil)           (str-pos 0) )      (while (and (< row-index (length
> row-nums)) (< str-pos 9))             ;; (comment) cycle through the
> change-strings          (while (< str-pos 9)                 ;; cycle
> through row-str, contributing to change-list                   (setq
> change-nums (append change-nums                                      
>             (list (string-to-number                                  
>                    (substring row-str str-pos (1+ str-pos)) )) )     
>                   row-str (nth row-index change-strings) )           
>   (setcar (car (nthcdr row-num pzl-form))                         
> (nth str-pos change-nums) )          (cl-incf str-pos) )     
>  (cl-incf row-index)       (cl-incf row-num-index) )   pzl-form ) )
-- 
Manuel Giraud

Re: Program problem

[Stephen Berman]

On Wed, 29 Nov 2023 00:16:46 +0000 (UTC) Lewis Creary via Users list for the GNU Emacs text editor <help-gnu-emacs@gnu.org> wrote:

> (It has been suggested to me by someone on the above mailing list thatthe
> following content would be appropriate for this list.)
> My purpose here is to discuss a bug I've discovered in an emacs lispfunction,
> fill-rows, that I've written (shown below).  The main ideaof this function is
> to help solvers of Sudoku puzzles who want to keeptrack of the numbers they've
> already entered into a puzzle.  This willhelp them to backtrack when they
> change some of those numbers.  It'simportant to understand the documentation
> string of this function(included in the function definition) before reading
> the code.

Your code appeared badly formatted, so I've reformatted it:

(defun fill-rows (row-nums change-strings pzl-form)
  "This fn takes as arguments a list of row indices, a list of an
equal number, s, of 9-digit strings, and a puzzle form (i.e, an
sexp of the form:

\((n n n n n n n n n)
  (n n n n n n n n n)
  (n n n n n n n n n)
  (n n n n n n n n n)
  (n n n n n n n n n)
  (n n n n n n n n n)
  (n n n n n n n n n)
  (n n n n n n n n n)
  (n n n n n n n n n)),

where n is a number between 0 and 9).  The fn returns as value
the result of filling in row m of the argument puzzle form with
numbers specified by the mth 9-digit string."
  (let* ((row-index 0)         
	 (row-num-index 0)
         (row-str (nth row-index change-strings))         
	 (row-num (nth row-num-index row-nums))
         (change-nums nil)         
	 (str-pos 0))
    ;; (comment) cycle through the change-strings
    (while (and (< row-index (length row-nums))
		(< str-pos 9))
        ;; cycle through row-str, contributing to change-list
      (while (< str-pos 9)
        (setq change-nums
	      (append change-nums          
                      (list (string-to-number              
			     (substring row-str str-pos (1+ str-pos)))))
              row-str (nth row-index change-strings))
        (setcar (car (nthcdr row-num pzl-form))
		(nth str-pos change-nums))
        (cl-incf str-pos))
      (cl-incf row-index)
      (cl-incf row-num-index))
    pzl-form))

There are a number of problems here.  One is that you increment str-pos
in the inner while-loop but don't reset it after leaving that loop, so
that when you start the second run through the outer loop, the condition
(< str-pos 9) is false and the function returns pzl-form after only one
run.  And you keep resetting row-str in the inner loop to the same
element of change-strings but don't change it at the end, so you never
get to the other strings in change-strings.  Another problem is you are
changing the same element of pzl-form each time through the outer loop,
because rownum doesn't change.  Also, in the setcar sexp, the first
argument is changing just the first element of the list because you take
its car, and the second argument changes it each time to the next
element in change-nums.

> I'll very much appreciate receiving any proposed bugfixes or other 
> suggestions you may have.

Here is a variant of your code that returns the result you want; rather
the an outer while-loop it uses dolist to loop directly over the
row-nums.  I've commented out unneeded parts of your original code:

(defun fill-rows (row-nums change-strings pzl-form)
  ""
  (let ((row-index 0)         
	;; (row-num-index 0)
        ;; (row-str (nth row-index change-strings))         
	;; (row-num (nth row-num-index row-nums))
	(row-str "")
        (change-nums nil)         
	(str-pos 0))
    ;; (comment) cycle through the change-strings
    (dolist (i row-nums)
      (setq row-str (nth row-index change-strings))
        ;; cycle through row-str, contributing to change-list
      (while (< str-pos 9)
        (setq change-nums
	      (append change-nums          
                      (list (string-to-number              
			     (substring row-str str-pos (1+ str-pos))))))
        (cl-incf str-pos))
      (setcar (nthcdr i pzl-form) change-nums)
      (setq str-pos 0
	    change-nums nil)
      (cl-incf row-index)
      ;; (cl-incf row-num-index)
      )
    pzl-form))

As food for thought and for you to experiment with, here is an
alternative implementation that uses some higher-level elisp functions
to avoid lower-level functions like setcar and nthcdr, and hence shorten
the code.  Also, it restructures the data into a form that seems to me
better suited to what you want, given your description.  The doc string
tries to explain that:

(defun fill-rows (dim rows)
  "Fill a square matrix of dimensions DIM with values from ROWS.
DIM is an integer specifying the number of rows and columns in
the matrix.  ROWS is an alist with elements (ROW . ELEMENTS),
where ROW specifies the (1-based) row number of the matrix and
ELEMENTS is a string whose characters are numerals specifying the
contents of the cells of ROW (in which each numeral in ELEMENTS
has been converted to the corresponding integer).  Each cell of
the rows not specified by ROWS is filled with 0.  Since the
result should be a square matrix, the lengths of ROWS and
ELEMENTS must equal DIM and each ROW must be an integer between 1
and DIM (inclusive)."
  (let (matrix)
    (dotimes (i dim)
      (let* ((rpair (assq (1+ i) rows))
	     (row (if rpair
		       (mapcar #'string-to-number
			       (mapcar #'string
				       (string-to-list (cdr rpair))))
		     (make-list dim 0))))
	(setq matrix (append matrix (list row)))))
    matrix))

Here's the invocation with your test data:

(fill-rows 9 '((1 . "123456789") (3 . "987654321") (5 . "123498765")))

I hope that was helpful.

Steve Berman

Re: Program problem

[Stephen Berman]

On Wed, 29 Nov 2023 17:20:30 +0100 Stephen Berman <stephen.berman@gmx.net> wrote:

[...]
> As food for thought and for you to experiment with, here is an
> alternative implementation that uses some higher-level elisp functions
> to avoid lower-level functions like setcar and nthcdr, and hence shorten
> the code.  Also, it restructures the data into a form that seems to me
> better suited to what you want, given your description.  The doc string
> tries to explain that:

I meant to add here that the last sentence of the doc string suggests
how to check for bad input, but I omitted adding such code for the sake
of brevity.

Steve Berman

> (defun fill-rows (dim rows)
>   "Fill a square matrix of dimensions DIM with values from ROWS.
> DIM is an integer specifying the number of rows and columns in
> the matrix.  ROWS is an alist with elements (ROW . ELEMENTS),
> where ROW specifies the (1-based) row number of the matrix and
> ELEMENTS is a string whose characters are numerals specifying the
> contents of the cells of ROW (in which each numeral in ELEMENTS
> has been converted to the corresponding integer).  Each cell of
> the rows not specified by ROWS is filled with 0.  Since the
> result should be a square matrix, the lengths of ROWS and
> ELEMENTS must equal DIM and each ROW must be an integer between 1
> and DIM (inclusive)."
>   (let (matrix)
>     (dotimes (i dim)
>       (let* ((rpair (assq (1+ i) rows))
> 	     (row (if rpair
> 		       (mapcar #'string-to-number
> 			       (mapcar #'string
> 				       (string-to-list (cdr rpair))))
> 		     (make-list dim 0))))
> 	(setq matrix (append matrix (list row)))))
>     matrix))
>
> Here's the invocation with your test data:
>
> (fill-rows 9 '((1 . "123456789") (3 . "987654321") (5 . "123498765")))
>
> I hope that was helpful.
>
> Steve Berman