CODEWITHSUNDEEP
clojure
Konstantin Milyutin
Konstantin Milyutin

Reputation: 12366

Remove element from a set only if element is in it

A list of players is defined as a set:

(def players (atom #{}))

Function that removes player should return different HTTP codes based on whether element was in the set or not:

(defn remove-player [player-name]
  (if (contains? @players player-name)
    (do (swap! players disj player-name)
        (status (response "") 200))
    (status (response "") 404)))

The problem with this code is that it might return multiple 200 to concurrent requests, even if only one request actually removed the element.

I guess I need to execute both contains? and disj atomically. Do I need to do explicit locking or is there a better way to do it?

Upvotes: 3

Views: 339

Answers (2)

ClojureMostly
ClojureMostly

Reputation: 4713

You can just add some more logic to your swapping function:

(for [el-rem [:valid-el :not-there]]
  (let [a (atom #{:valid-el :another-one})
        disj-res
        (swap! a
               (fn [a]
                 (with-meta (disj a el-rem)
                            {:before-count (count a)})))]
    [disj-res
     "removed:"
     el-rem
     (not (== (:before-count (meta disj-res))
              (count disj-res)))]))

You then compare the count of the return value disj-res and the count in the meta data. If it differs, then disj did remove an element. If not, the element was not present.

Upvotes: 2

leetwinski
leetwinski

Reputation: 17859

the swap! itself is atomic operation, so inside the swap's calculating function you can be sure that the first parameter (atom's current value) is consistent. Personally i would make a helper function for that like this:

(defn remove-existent [value set-a]
  (let [existed (atom false)]
    (swap! set-a
           #(if (contains? % value)
              (do (reset! existed true)
                  (disj % value))
              (do (reset! existed false) 
                  %))
    @existed))

as you can see, lambda expression contains both existency check and removal.

user> (def players (atom #{:user1 :user2}))
#'user/players

user> (remove-existent :user100 players)
false

user> (remove-existent :user1 players)
true

user> @players
#{:user2}

update

Inspired by @clojuremostly's excellent metadata approach, you can make it much better:

(defn remove-existent [value set-a]
  (-> (swap! set-a #(with-meta (disj % value)
                      {:existed (contains? % value)}))
      meta
      :existed))

Upvotes: 3

Related Questions