Clojure

Add deref-swap! and deref-reset! (swap! and reset! that return prior value)

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Critical Critical
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Prescreened

Description

Sometimes, when swapping or resetting an atom, it's desirable to know the value before the update. The existing swap! and reset! functions return the new value instead. Currently, the only option is to roll your own using a loop and compare-and-set!

Example use cases:

  • When an atom contains a PersistentQueue and you want to atomically remove the head of the queue and process it: if you run (swap! q pop), you have lost the reference to the old head of the list so you can't process it.
  • Want to check if an operation has occurred before by using atom as a flag (this can be achieved with compare-and-set! but reads a little easier this way).
    (def has-run-once (atom false))
    ...
    (when-not (get-and-set! has-run-once true)
    (do something))
  • Want to use an atom similarly to a java.util.concurrent.LinkedTransferQueue, for the case of pairing up adds by writers and drainTo y readers:
    Thread 1: (swap! atm conj item1)
    Thread 2: (swap! atm conj item2)
    Thread 3: (let [new-vals (get-and-set! atm [])] 
    (do-something new-vals))

Approach:

  • Extends clojure.lang.IAtom with derefReset and derefSwap with the exact same arities as their original variants, .reset and .swap. The deref-methods returns the old (deref-ed) value of the atom after a successful mutation of the atom.
  • Added clojure.core/deref-reset! - just like reset! it unconditionally changes values of atoms but returns
    the old atom value. Based on Steven Yi's get-and-set! in ticket CLJ-1599, patch named get-and-set.diff
  • Added clojure.core/deref-swap! - just like swap! but returns the old atom value.

Patch: deref-swap-deref-reset-with-IAtomDeref-and-tests.diff

Activity

Hide
Philip Potter added a comment - - edited

Overtone already defines functions like this in overtone.helpers.ref, which get used by overtone.libs.event. These return both the old and the new value, although in all existing use cases only the old value gets used.

flatland/useful defines a trade! fn which returns the old value, although the implementation is less clean than a compare-and-set! based solution would be.

Show
Philip Potter added a comment - - edited Overtone already defines functions like this in overtone.helpers.ref, which get used by overtone.libs.event. These return both the old and the new value, although in all existing use cases only the old value gets used. flatland/useful defines a trade! fn which returns the old value, although the implementation is less clean than a compare-and-set! based solution would be.
Hide
Philip Potter added a comment -

Chris Ford suggested "swap-out!" as a name for this function. I definitely think "swap-returning-old!" isn't the ideal name.

Show
Philip Potter added a comment - Chris Ford suggested "swap-out!" as a name for this function. I definitely think "swap-returning-old!" isn't the ideal name.
Hide
Jozef Wagner added a comment -

I propose a switch! name. The verb switch is defined as "substitute (two items) for each other; exchange.", and as you get the old value back, it evokes slightly the exchange of items.

Show
Jozef Wagner added a comment - I propose a switch! name. The verb switch is defined as "substitute (two items) for each other; exchange.", and as you get the old value back, it evokes slightly the exchange of items.
Hide
Philip Potter added a comment -

Medley also has a deref-swap! which does the same thing.

Show
Philip Potter added a comment - Medley also has a deref-swap! which does the same thing.
Hide
Alex Miller added a comment -

I think deref-swap! seems like a morally equivalent name to Java's AtomicReference.getAndSet() which is the same idea.

Show
Alex Miller added a comment - I think deref-swap! seems like a morally equivalent name to Java's AtomicReference.getAndSet() which is the same idea.
Hide
Philip Potter added a comment -

Funny you say that Alex, because prismatic/plumbing defines a get-and-set! (also defined by other projects), equivalent to deref-reset! in medley. Plumbing also defines swap-pair! which returns both old and new values, like the overtone fn, although once again the only usage I can find only uses the old value.

Show
Philip Potter added a comment - Funny you say that Alex, because prismatic/plumbing defines a get-and-set! (also defined by other projects), equivalent to deref-reset! in medley. Plumbing also defines swap-pair! which returns both old and new values, like the overtone fn, although once again the only usage I can find only uses the old value.
Hide
Alex Miller added a comment -

I think it's important to retain the notion that you are not switching/exchanging values but applying the update model of applying a function to the old value to produce the new value. I don't even particularly like "swap!" as I think that aspect is lost in the name (alter and alter-var-root are better). I like that "deref-swap!" combines two words with existing connotations and orders them appropriately.

Show
Alex Miller added a comment - I think it's important to retain the notion that you are not switching/exchanging values but applying the update model of applying a function to the old value to produce the new value. I don't even particularly like "swap!" as I think that aspect is lost in the name (alter and alter-var-root are better). I like that "deref-swap!" combines two words with existing connotations and orders them appropriately.
Hide
Timothy Baldridge added a comment -

except that that naming doesn't fit well compared to functions like nfirst which are defined as (comp next first). This function is not (comp deref swap!).

Show
Timothy Baldridge added a comment - except that that naming doesn't fit well compared to functions like nfirst which are defined as (comp next first). This function is not (comp deref swap!).
Hide
Linus Ericsson added a comment - - edited

The patch deref-swap-deref-reset-extending-IAtom.diff addresses CLJ-1454 and CLJ-1599 "Add deref-reset! (reset! that returns old value)".

The patch extends IAtom with .derefSwap and .derefReset methods with identical signatures as to .swap and .reset but are intended to return the derefed (old) value of the atom upon successful mutation of the atom.

The commit message (could be used as an updated description of this ticket, or a new one)

Extends clojure.lang.IAtom with derefReset and derefSwap with the
exact same arities as their original variants, .reset and .swap.

The deref-methods returns the old (derefed) value of the atom after
a successful mutation of the atom.

Added functions:

clojure.core/deref-reset!
Just like reset! it unconditionally changes values of atoms but returns
the old atom value.

clojure.core/deref-swap!
Just like swap! but returns the old atom value.

The deref-reset is based on Steven Yi's get-and-set! in ticket CLJ-1599,
patch named get-and-set.diff

Show
Linus Ericsson added a comment - - edited The patch deref-swap-deref-reset-extending-IAtom.diff addresses CLJ-1454 and CLJ-1599 "Add deref-reset! (reset! that returns old value)". The patch extends IAtom with .derefSwap and .derefReset methods with identical signatures as to .swap and .reset but are intended to return the derefed (old) value of the atom upon successful mutation of the atom. The commit message (could be used as an updated description of this ticket, or a new one) Extends clojure.lang.IAtom with derefReset and derefSwap with the exact same arities as their original variants, .reset and .swap. The deref-methods returns the old (derefed) value of the atom after a successful mutation of the atom. Added functions: clojure.core/deref-reset! Just like reset! it unconditionally changes values of atoms but returns the old atom value. clojure.core/deref-swap! Just like swap! but returns the old atom value. The deref-reset is based on Steven Yi's get-and-set! in ticket CLJ-1599, patch named get-and-set.diff
Hide
Alex Miller added a comment -

Review:

  • remove ":static true" in the new fns, this is dead
  • as mentioned on the mailing list, I think we need to consider that we are introducing a breaking change here. Rather than modifying IAtom we should add the methods in a new interface IAtomDeref that Atom implements. The Clojure functions should then type-hint to IAtomDeref instead.
Show
Alex Miller added a comment - Review:
  • remove ":static true" in the new fns, this is dead
  • as mentioned on the mailing list, I think we need to consider that we are introducing a breaking change here. Rather than modifying IAtom we should add the methods in a new interface IAtomDeref that Atom implements. The Clojure functions should then type-hint to IAtomDeref instead.
Hide
Alex Miller added a comment -

Oh, and needs tests!

Show
Alex Miller added a comment - Oh, and needs tests!
Hide
Linus Ericsson added a comment -

Alex, in deref-swap-deref-reset-with-IAtomDeref-and-tests.diff you'll find a patch with the differences

  • removed ":static true"
  • added an IAtomDeref interface instead of extending IAtom
  • 2 test cases for deref-reset!
  • 1 test case for deref-swap!

Please especially have a look at the test-cases. I tried to catch the various arities of deref-swap! but I don't know if binding warn-on-reflection true actually affects the test cases in any way. Suggestions on how to test wether "arity works" - if even worth the effort to test - are most welcome.

Given the nature of atoms, a suite of generative tests would probably be the best idea. Until then, simple functional tests for the other atom functions should be added, but that's for another ticket.

Show
Linus Ericsson added a comment - Alex, in deref-swap-deref-reset-with-IAtomDeref-and-tests.diff you'll find a patch with the differences
  • removed ":static true"
  • added an IAtomDeref interface instead of extending IAtom
  • 2 test cases for deref-reset!
  • 1 test case for deref-swap!
Please especially have a look at the test-cases. I tried to catch the various arities of deref-swap! but I don't know if binding warn-on-reflection true actually affects the test cases in any way. Suggestions on how to test wether "arity works" - if even worth the effort to test - are most welcome. Given the nature of atoms, a suite of generative tests would probably be the best idea. Until then, simple functional tests for the other atom functions should be added, but that's for another ticket.
Hide
Linus Ericsson added a comment -

(the second patch is independent from the first one)

Show
Linus Ericsson added a comment - (the second patch is independent from the first one)
Hide
Alex Miller added a comment -

Looks good to me, marking prescreened.

Show
Alex Miller added a comment - Looks good to me, marking prescreened.

People

Vote (9)
Watch (5)

Dates

  • Created:
    Updated: