bug#62119: 29.0.60; Add map-insert!, eventually get rid of map-not-inplace error

[Michael Heerdegen]

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.