how to avoid destructuring-bind ARG-COUNT-ERROR?

Question

I need to read a string of Common Lisp object from database. The object should be a list with two elements of double-float; "(1.0d0 2.0d0)" for example:

(let* ((str "(1d0 2d0)")
       (off (read-from-string str)))
  (destructuring-bind (x y)
      off (list x y)))

But there are conditions that the string is not properly formatted? For example, the query failed, or the object doesn't exist. The code will give arg-count-error:

error while parsing arguments to DESTRUCTURING-BIND:
  too few elements in
    ()
  to satisfy lambda list
    (X Y):
  exactly 2 expected, but got 0
   [Condition of type SB-KERNEL::ARG-COUNT-ERROR]

I have to use the following code to do type-check.

(let* ((str "(1d0 2d0)")
       (off (read-from-string str)))
  (destructuring-bind (x y)
      (if (and off
               (typep off 'list)
               (= 2 (length off)))
          off
          (list 0d0 0d0)) (list x y)))

The code snippet will return the default value if the str is not properly formatted. : (0.0d0 0.0d0);

Am I doing it right? Is there an better way to avoid this error?

Answer 1

There are a few ways to go about this. One option would be to use a pattern matching library (such as Trivia).

(defun read-coord (string)
  (match (read-from-string string nil)
    ((list x y) (list x y))
    (_ (list 0d0 0d0))))

CL-USER> (read-coord "(1d0 2d0)")
(1.0d0 2.0d0)
CL-USER> (read-coord "(1d0)")
(0.0d0 0.0d0)
CL-USER> (read-coord "")
(0.0d0 0.0d0)

If you want to check that X and Y are floats, you can add a guard pattern

(defun read-coord (string)
  (match (read-from-string string nil)
    ((list (guard x (floatp x))
           (guard y (floatp y)))
     (list x y))
    (_ (list 0d0 0d0))))

As pointed out by Rainer Joswig in his answer, READ-FROM-STRING can cause other errors and *READ-EVAL* should be set to NIL when reading.

(defun safely-read-from-string (string)
  (let ((*read-eval* nil))
    (ignore-errors (read-from-string string))))

(defun read-coord (string)
  (match (safely-read-from-string string)
    ((list (guard x (floatp x))
           (guard y (floatp y)))
     (list x y))
    (_ (list 0d0 0d0))))

Also remember that you can use lambda list keywords, such as &OPTIONAL, with DESTRUCTURING-BIND. Using Alexandria for the ENSURE-LIST-function, you could write

(defun read-coord (string)
  (destructuring-bind (&optional (x 0d0) (y 0d0) &rest _)
      (ensure-list (safely-read-from-string string))
    (declare (ignore _))
    (list x y)))

CL-USER> (read-coord "(1d0 2d0)")
(1.0d0 2.0d0)
CL-USER> (read-coord "(1d0)")
(1.0d0 0.0d0)
CL-USER> (read-coord "")
(0.0d0 0.0d0)

If you don't want to use any libraries, you can just check that a list is given yourself

(defun read-coord (string)
  (let ((coord (safely-read-from-string string)))
    (destructuring-bind (&optional (x 0d0) (y 0d0) &rest _)
        (if (listp coord)
            coord
            (list coord))
      (declare (ignore _))
      (list x y))))

Answer 2

I am relatively new to lisp, so you better wait for the old dogs to give you their solution. Here is how I would refactor it.

For starters you can also check if both elements of a list are of type floats with

(floatp element)

This will make your snippet look like this

(let* ((str "(1d0 2d0)")
       (off (read-from-string str)))
  (destructuring-bind (x y)
    (if (and off
             (typep off 'list)
             (= 2 (length off)
             (every #'floatp off)) ; here is the new code
        off
        (list 0d0 0d0)) (list x y)))

However it is quite hard on the eyes already. Let's make it a predicate. I will assume that what you want is a coordinate. You will make the variable name match your application domain.

(defun coord-p (coord)
  (and coord
       (typep coord 'list)
       (= 2 (length coord)
       (every #'floatp coord)))

Now the snippet should look like this:

(let* ((str "(1d0 2d0)")
       (coord (read-from-string str)))
  (destructuring-bind (x y)
    (if (coord-p coord)
        coord
        (list 0d0 0d0))
    (list x y)))

Let's simplify the check inside the destucturing-bind

(defun ensure-coord (coord)
   (if (coord-p coord)
       coord
       (list 0d0 0d0)))

Now the snippet should look like this:

(let* ((str "(1d0 2d0)")
       (coord (read-from-string str)))
  (destructuring-bind (x y)
      (ensure-coord coord)
    (list x y)))

Now let's put the snippet inside a function:

(defun coord-from-string (str)
  (destructuring-bind (x y)
      (ensure-coord
       (read-from-string str))
    (list x y)))

This looks quite alright to me. But actually... why do we need the destructuring-bind again? It is completely useless by now:

(defun coord-from-string (str)
  (ensure-coord
    (read-from-string str))

Bonus tip: Use defstruct with (:type list).

For further clarity and expressiveness you can declare a defstruct for your object. E.g.:

(defstruct (coord (:type list))
   (x 0d0 :type double-float)
   (y 0d0 :type double-float))

This way every list of two elements can be operated with the function provided from Common Lisp for operation of structs. E.g.:

(coord-y '(1d0 2d0)) ; => 2.0d0

Note the (:type list) in the declaration. This is why we can use a simple list to act as a struct. This way we can combine the versatility of lists with the functionality and expressiveness of structs. For example if you follow this tip the ensure-coord function will look like this:

(defun ensure-coord (coord)
  (if (coord-p coord)
      coord
      (make-coord)))

Arguably, the intentions are clearer.

Answer 3

Note that there are many sources of possible errors. For example the reader could detect errors.

Make sure that you handle errors:

(defun get-it (s)
  (let ((*read-eval* nil))      ; don't execute code on reading!
    (flet ((check (it)
             (if (and (listp it)
                      (= (length it) 2)
                      (every #'double-float-p it))
                 it
               (error "data is not a list of two double-floats: ~a" it))))
             (handler-case (check (read-from-string s))
               (error (condition)
                 (princ condition)
                 (list 0.0d0 0.0d0))))))


CL-USER 34 > (get-it "(0.0d0 0.0d0)")
(0.0D0 0.0D0)

CL-USER 35 > (get-it "(0.0d0 0.0d0")
End of file while reading stream #<SYSTEM::STRING-INPUT-STREAM 40E06AD7DB>.
(0.0D0 0.0D0)

CL-USER 36 > (get-it "(0.0d0 foo:aa))")
Reader cannot find package FOO.
(0.0D0 0.0D0)

CL-USER 37 > (get-it ")")
Unmatched right parenthesis.
(0.0D0 0.0D0)

CL-USER 38 > (get-it "(1 2 3)")
data not a list of two double-floats: (1 2 3)
(0.0D0 0.0D0)