Should I run checks outside or inside my swap! function?

Question

When I'm going to swap! the value of an atom conditionally, should the condition wrap the swap! or should it be part of the function swap! calls?

(import '(java.time Instant))

(def not-nil? (comp not nil?))

(defonce users (atom {
  "example user 1" {:ts (Instant/now)}
  "example user 2" {:ts (Instant/now)}
}))

(defn update-ts [id]
  (if (not-nil? (get @users id))
    (swap! users assoc-in [id :ts] (Instant/now))))

In the above example, I'm doing the existence check for the user before doing the swap!. But couldn't the user be deleted from users after the check but before the swap!? So, then, is it safer to put the check inside the function run by swap!?

(defn update-ts [id]
  (swap! users (fn [users]
                   (if (not-nil? (get users id))
                       (assoc-in users [id :ts] (Instant/now))
                       users))))

I think the condition should wrap the swap!, also it's going to be easy when debugging. — Ertuğrul Çetin, Jun 12 '17 at 23:04
@ErtuğrulÇetin That was my natural instinct as seen in my original code. But I don't want the atom's state to change between the check and this particular `swap!`. — , Jun 12 '17 at 23:13
You might consider configuring your atom with a validator function, if you don't think putting the validation inside the function passed to `swap!` is natural. — Charles Duffy, Jun 12 '17 at 23:28
While occasionally a validator can be useful, I don't see how a validator would help here. The question is not about what final states of the atom are valid, but what state transitions are valid for a particular call to `swap!`. Specifically, it is invalid to transition from a non-nil timestamp to any other timestamp value. There's really nowhere to put this check but in the `swap!` itself. — amalloy, Jun 12 '17 at 23:45

score 6 · Accepted Answer · answered Jun 12 '17 at 23:17

But couldn't the user be deleted from users after the check but before the swap!? So, then, is it safer to put the check inside the function run by swap!?

Yes, exactly right. You should never make a decision about how to mutate an atom from anywhere but inside of a swap! on that atom. Since swap! is the only operation guaranteed to be atomic, every time you do otherwise (ie, make a decision about an atom from outside of a swap! on it), you introduce a race condition.

Alan Thompson · Answer 2 · 2017-06-13T14:01:23.950

But couldn't the user be deleted from users after the check but before the swap!? So, then, is it safer to put the check inside the function run by swap!?

As amalloy said, if you need it to be bulletproot you must put the not-null? check inside the swap function.

However, please keep in mind that you & your team are writing the rest of the program. Thus, you have a lot of outside information that may simplify your decision:

If you only ever have one thread (like most programs), you never need to worry about a race condition.
If you have 2 or more threads, maybe you never remove entries from the map (it only accumulates :ts values). Then you also don't need to worry about a conflict.
If your function is more complicated than the simple example above, you may wish to use a (dosync ...) form to wrap multiple steps instead of shoehorning everything in to a single swap function.

In the third case, replace the atom with a ref. An example is:

(defonce users (ref {...} )) ; ***** must us a ref here *****
(dosync
  (if (not-nil? (get @users id))
    <lots of other stuff...>
    (alter users assoc-in [id :ts] (Instant/now)))))

That's actually a pretty good argument for a ref instead of an atom. I have found this answer helpful for explaining the differences. https://stackoverflow.com/a/9136699/457586 — , Jun 13 '17 at 02:22

Should I run checks outside or inside my swap! function?

2 Answers2