update

[gfredericks]

Would a pull request for (defn update [m k f & args] (apply update-in m [k] f args)) be accepted?

Every time I use update-in with a single key, I imagine a parenthesis losing its wings somewhere.

[tcrayford]

@fredericksgary : rewrite it not to use update-in, and I'd be a super +1 for this, I end up carting this fn around all my projects.

[gfredericks]

Yeah I thought that might get a comment :). update-in is obviously not the best impl, just the clearest explanation of what it does.

[w01fe]

Hrm, I've gone back and forth on this one myself a bit.

While assoc-in and dissoc-in have counterparts assoc and dissoc, those versions also bring new utility: being able to assoc or dissoc multiple keys in a single call. Because of the & args, you can't do the same thing with update, which breaks the parallelism. Personally I've just grown used to (update-in m [k] f). I'll talk it over with the team and see what they think, and see if anyone else here has a strong opinion...

[tcrayford]

Often I'm using update under relatively high GC pressure, and calling it a tonne, so not doing an extra allocation on every call is nice.

[gfredericks]

assoc-in and dissoc-in could have the multiple-keys-in-a-single-call feature if they'd been written that way. While neither update-in nor update can (though you can imagine a different version without & args that takes multiple pairs -- just seems less useful).

I think the only reason to have any of the flat versions is a bit of sugar for the most common case, and a bit of performance.

[w01fe]

Good points -- that all makes sense. I think my last reservation is that we encourage :use of plumbing.core, and since update is a pretty generic term this will cause warnings / issues if people already have update methods defined in their namespaces. I looked through our codebase and we only have one such instance, so maybe it's not a big deal. Does anyone else have data points on this?

[gfredericks]

Would it be weird to put it in plumbing.map?

[w01fe]

Wouldn't be weird, but on the whole core seems like a more fitting place.
As long as nobody objects to adding it to core for the above reason, let's
just do that.

On Wed, Mar 5, 2014 at 5:30 AM, fredericksgary notifications@github.comwrote:

Would it be weird to put it in plumbing.map?

Reply to this email directly or view it on GitHubhttps://github.com//issues/20#issuecomment-36741736
.

[gfredericks]

I just searched internally 11 codebases (5 libs & 6 apps) totalling ~20,000 LOC, and there were 2 instances of defn update that weren't the version proposed here.

I'm more prone to use :refer [...] than :use so that kind of conflict doesn't bother me, but thought I'd supply the requested data points.

[w01fe]

Cool, thanks -- PR away :).

On Wed, Mar 5, 2014 at 11:46 AM, fredericksgary notifications@github.comwrote:

I just searched internally 11 codebases (5 libs & 6 apps) totalling
~20,000 LOC, and there were 2 instances of defn update that weren't the
version proposed here.

I'm more prone to use :refer [...] than :use so that kind of conflict
doesn't bother me, but thought I'd supply the requested data points.

Reply to this email directly or view it on GitHubhttps://github.com//issues/20#issuecomment-36784865
.

[tcrayford]

note: I first saw this function here:

• https://github.com/Factual/skuld/blob/master/src/skuld/util.clj#L87

[gfredericks]

Created #21