[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: New lisp library -- cell.el
From: |
Stefan Monnier |
Subject: |
Re: New lisp library -- cell.el |
Date: |
Fri, 28 Feb 2020 10:43:07 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
Some comments on the code:
> (defsubst cell-option-none ()
> "Create an option in None variant."
> cell-option--none-singleton)
Why not use nil for "none"? That would simplify your code and most
likely would make for much more natural/idiomatic code in the clients of
your library.
[ Traditionally in Lisp when you need something like a "option` (aka
`Maybe`) type, you use `(cons X nil)` for `some X` and `nil` for `none`. ]
> (cl-defstruct (cell-box
> (:constructor nil)
> (:constructor cell-box--make (-inner))
> (:copier nil))
> -inner)
Rather than put the additional "-" in the slot name, you can use
`(:conc-name call-box-)`.
> (defalias 'cell-box-make 'cell-box--make
> "Create a single mutable unit with INITIAL-VALUE.")
>
> (defalias 'cell-box-inner 'cell-box--inner
> "Return the inner value of BOX.
> \n(fn BOX)")
Why bother with those indirections?
> (defun cell-weak-get (cell)
> "Return Some(inner) if reference in CELL still alive, otherwise return
> None."
> (let* ((gensym (make-symbol "not-found"))
> (val (gethash t (cell-weak--inner-table cell) gensym))
> (found (not (eq gensym val))))
> (if found
> (cell-option-some val)
> (cell-option-none))))
That makes every call to `cell-weak-get` allocate a new symbol only to
throw it away. I'd recommend you define a top-level constant
(defconst cell--not-found (make-symbol "not-found"))
instead. There are other solutions to this problem that might be even
better, tho. E.g. you could compute `found` as (< 0 (hash-table-count ...).
Better yet, disallow weak cells containing `nil` (since `nil` can't be
GC'd anyway it doesn't make much sense to put it inside a weak-reference)
and you don't need this at all:
(defun cell-weak-make (inner)
"Create a cell takes a weak reference of INNER."
(if (null inner)
nil ;; We could also signal an error, but returning nil
;; allow us to treat nil as its own weak-reference,
(let* ((cell (cell-weak--internal-make))
(internal-ht (cell-weak--inner-table cell)))
(puthash t inner internal-ht)
cell)))
(defun cell-weak-get (cell)
"Return inner if reference in CELL still alive, otherwise return nil."
(when cell ;; If nil, it's a weak ref to nil.
;; The stored content is never nil, so it only returns nil if
;; the content was GC'd.
(gethash t (cell-weak--inner-table cell))))
-- Stefan