[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#62119: 29.0.60; Add map-insert!, eventually get rid of map-not-inpla
From: |
Michael Heerdegen |
Subject: |
bug#62119: 29.0.60; Add map-insert!, eventually get rid of map-not-inplace error |
Date: |
Wed, 15 Mar 2023 04:31:23 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Augusto Stoffel <arstoffel@gmail.com> writes:
> In my understanding, map.el should operate on values, like a
> functional language, an not on objects (a.k.a. identities or places)
> like procedural scripting languages.
I guess some people will disagree that we should make that a core
assumption in this library.
> So I suggest adding the following:
>
> (cl-defgeneric map-insert! (map key value)
> "Return a new map like MAP except that it associates KEY with VALUE.
> This may destructively modify MAP to produce the new map."
> (map-insert map key value))
>
> (cl-defmethod map-insert! ((map list) key value)
> (if (map--plist-p map)
> (if-let ((tail (memq key map)))
> (prog1 map
> (setf (cadr tail) value))
> `(,key ,value ,@map))
> (if-let ((pair (assoc key map)))
> (prog1 map
> (setf (cdr pair) value))
> `((,key . ,value) ,@map))))
>
> (cl-defmethod map-insert! ((map hash-table) key value)
> (puthash key value map))
But I like this idea.
> I would then suggest to make map-put! obsolete, due to its limitation
> and conceptual confusion. Also, the docstring could be clarified like
> this:
>
> (cl-defgeneric map-put! (obj key value &optional testfn)
> "Associate KEY with VALUE in the map OBJ.
> If KEY is already present in OBJ, replace the associated value
> with VALUE.
> This operates by modifying OBJ in place. OBJ must be an object that
> can be modified in place; otherwise signal a `map-not-inplace' error."
> ;; `testfn' only exists for backward compatibility with `map-put'!
> (declare (advertised-calling-convention (map key value) "27.1")))
I don't like renaming the first argument to OBJ (you forgot to change
the calling convention btw). Using a sensible name for the first
argument is important. Apart from that I found the original version of
the docstring not worse or less clear than yours.
> As to the naming of map-insert!, we should first decide if the
> exclamation mark is the convention we want for destructive operations.
Personally I would rather expect that a *!-named function modifies
something in some location and the return value is not the important
part.
> One issue here is that map-delete is destructive. In fact, there should
> be a destructive and a non-destructive version of that method too.
I like your `map-insert!', and this will also have advantages. I'm not
sure we want to give up the old generics (like `map-put!') however. I'm
also not sure if it would be worth it to have both at the same time,
since the cost is that we need to have separate implementations for all
generics.
I how others think about this suggestions.
Thanks,
Michael.