Welcome! Please see the About page for a little more info on how this works.

+5 votes
in Clojure by

There is no clojure.core/dissoc-in although there is an assoc-in.
It is correct that dissoc-in can be build with update-in and dissoc but this is an argument against assoc-in as well.
When a shortcut for assoc-in is provided, there should also be one for dissoc-in for consistency reasons.
Implementation is analogical to assoc-in.

16 Answers

+1 vote
by

Comment made by: alexmiller

We must consider vectors or associative data structures other than maps. I also would prefer not to have a flag and to instead leave it open for variadic to remove multiple.

With an equivalent like:

(update-in [{:foo 1}] [0] dissoc :foo)

we would be left with {{[{}]}} so the existing way to do this does leave empty collections.

I also looked at some of the existing dissoc-in implementations that already exist - core.incubator, taoensso.encore, clj-http, medley, useful, plumbing, etc. Most work with a single key-path and leave empty collections - these seem largely to derive from the incubator version. The encore one works with multiple paths by separating the key path from the final leaf keys to dissoc (variadic). The medley, useful, and plumbing ones are map-only and remove empty maps.

I can't imagine that we would make a map-specific version of this in core (since all the other -in functions are generic) and thus I conclude that we would need a dissoc-in that left empty collections. Given that, I'm not sure whether dissoc-in provides enough value over update-in + dissoc to be worth adding to core at this point, especially since it exists in a variety of utility libraries (in many cases with differing semantics). We are then potentially breaking (or at least inconveniencing) many users of existing dissoc-in impls. I'm going to leave this open for comments, but I'm of a mind to decline this enhancement at this point.

+1 vote
by

Comment made by: alexmiller

True, although I'd say the factors balance differently in the update case. With update there fewer implementations and they matched what we were proposing to add. So, they created the potential for the existing ones to be replaced with a core function.

I think the common utility library impls (flatland/useful/plumbing) are actually covering a need that is different - update-in + dissoc + path removal on maps only. So if the goal is to cover a common usage, then we would need to have it work only on maps and remove empty paths left behind. However, I think that is somewhat at odds with the other core -in functions. I guess maybe there is a middle ground to do path removal but make it work beyond maps - that would satisfy the needs indicated by the utility libs but also be generic like the other -in functions. And if you needed to leave the empty colls, you could do what you do now - update-in+dissoc.

0 votes
by

Comment made by: jafingerhut

Patch clj-1063-add-dissoc-in-patch-v2.txt dated Sep 13 2012 supersedes 001-dissoc-in.diff dated Sep 7 2012. It fixes a typo (missing final " in doc string), and adds a test case for the new function.

0 votes
by

Comment made by: bronsa

Thanks for the fix Andy

0 votes
by

Comment made by: jafingerhut

This proposed dissoc-in should be compared with the one in clojure.core.incubator which I just happened across. I see they look different, but haven't examined to see if there are any behavior differences.

https://github.com/clojure/core.incubator/blob/master/src/main/clojure/clojure/core/incubator.clj

0 votes
by
_Comment made by: bronsa_

dissoc-in in clojure.core.incubator recursively removes empty maps

user=> (clojure.core.incubator/dissoc-in {:a {:b {:c 1}}} [:a :b :c])
{}

while the one in this patch doesn't (as I would expect)

user=> (dissoc-in {:a {:b {:c 1}}} [:a :b :c])
{:a {:b {}}}
0 votes
by

Comment made by: stu

Please do this work in the incubator.

0 votes
by

Comment made by: andrewhr

Keeping the empty paths is really the most expected thing to do? I say, since both {{assoc-in}}/{{update-in}} create paths along the way this behavior looks to be the real "dual".

What kind of bad things could happen that {{nil}} punning does not deal well with it? Those issues would be too much obscure for {{dissoc-in}} user's point of view?

0 votes
by

Comment made by: alexmiller

Tests are much too light in the patch. Should test multiple levels, records, vectors, etc.

0 votes
by
_Comment made by: desk@danielcompton.net_

Testing "clj-1063-add-dissoc-in-patch-v2.txt"


(dissoc-in {} [:a :b :c])
=> {:a {:b nil}}


this behaviour is pretty unexpected. The incubator version doesn't have this problem, though it does remove empty maps.

I'm working on tests (and possibly a new dissoc-in) for this patch, but I'm not clear yet whether empty paths should be kept or removed? I don't mind either way, though I tend to agree with Andrew Rosa that keeping them is more symmetrical.
0 votes
by

Comment made by: desk@danielcompton.net

@alex, do you want empty paths to be kept or removed? Either way is acceptable, though on balance I slightly prefer leaving them. I can work up tests for this, once I know which direction you want to go in.

0 votes
by

Comment made by: alexmiller

What are the tradeoffs of one vs the other?

In places where people are using dissoc-in stand-ins (update-in with dissoc etc) what behavior are they expecting?

0 votes
by
_Comment made by: michalmarczyk_

If we only wanted to handle maps nested in maps …, I'd probably prefer removing empty subpaths.

However, if we want to admit vectors along the way, then leaving empty paths in place makes for pretty clear semantics in that case:


(dissoc-in [{:foo 1}] [0 :foo])
;= [{}]


In contrast, removing empty paths would seem to lead to problems that would have to be resolved either by limiting the path-removing behaviour to maps or throwing when a subpath nested in a vector becomes entry. Either approach seems suboptimal.
0 votes
by

Comment made by: michalmarczyk

Also we could technically accept a flag to decide whether empty paths should be removed and let the user ensure that they only pass {{true}} if there are no vectors in the way. I'm not sure I like this approach very much, though, because it seems to me that the more natural purpose for extra arguments is to provide additional paths to dissoc for matching variadic {{dissoc}} and {{dissoc-in}} semantics (cf. CLJ-1771).

0 votes
by

Comment made by: bronsa

The same argument could've been made for the inclusion of update. The fact that both update and dissoc-in can be found in several util libraries should indicate that people are not ok with just using update-in + dissoc.

Sure adding it to core is going to cause warnings when using some libraries (it's not going to break anything, the AOT bugs that caused breakages with update have been fixed) but I don't think that's really a big deal; new versions of those libraries will get released quickly and even if that's not the case a warning is not the end of the world.

...