CODEWITHSUNDEEP
clojure
user193116
user193116

Reputation: 3568

In clojure , what is the idiomatic way to perform a serie of test functions on data , which each function having a sideeffect?

Let us suppose we are supposed to do an series of checks on email address and the data that is associated with the email:

  1. Check if the email is nil ? if nil return {:msg "emial is nil"}

  2. check if email has an account (i.e do a db lookup of the account for email). if account is nil , return {:msg "no account"}

  3. check if the account is in expired. if accout is not in expired , return {:msg expired}

.. and so on

Currrently, I have coded this as nested if-let statements:
         (if-let [email (if (nil? email) nil email)]
               (if-let [acct (load-act email)]
                   (if-let [goodacct (if (expired? acct) nil acct)]    
                ....        
))

is there a nicer way or more idiomatic way to write this ?

Upvotes: 1

Views: 117

Answers (3)

Denis Fuenzalida
Denis Fuenzalida

Reputation: 3346

Here's another approach: you model your user account as a plain map, and you define some basic validation functions. If it's just the first error that you are interested in, you can map the application of different functions across the same user account data, and pick the first error.

You can enhance this to collect all of the errors in a vector, or augmenting your map (eg. with reduce) so steps further down on the validation pipeline can re-use data obtained from side-effects from prior stages (eg. if a prior step went to the db, maybe assoc more keys into the map so that later validations can re-use the same data without extra queries. You'd need to change the validation functions to take a map and return a pair of [updated-map error]).

A quick and dirty example:

(defn valid-email [{:keys [email]}]
  (when-not email "email is nil"))

(defn valid-account [{:keys [account]}]
  (when-not account "no account"))

(defn expired-account [{:keys [account]}]
  (when (:expired account) "account expired"))

(def validations [valid-email valid-account expired-account])

(defn validator [user-data]
  (when-let [error (some identity (map #(% user-data) validations))]
    {:msg error}))

;; (validator {})
;; => {:msg "email is nil"}

;; (validator {:email "[email protected]"})
;; => {:msg "no account"}

;; (validator {:email "[email protected]" :account {:expired true}})
;; => {:msg "account expired"}

;; (validator {:email "[email protected]" :account {:expired false}})
;; => nil ;; nil means "no validation errors"

Upvotes: 1

Gwang-Jin Kim
Gwang-Jin Kim

Reputation: 9865

The same like @xificurC's last version. But why not use normal syntax? (no need for unnecessary dependencies ;) ). Is not that more typing:

(cond (nil? email) {:msg "email is nil"}
      :else (let [acct (load-act email)]
              (cond (nil? acct) {:msg "no account"}
                    (expired? acct) {:msg "expired account"}
                    :else acct)))

Of course, one could discuss whether first cond should be an if. But I like cond, because it lays down horizontally cause and effect on the left and right hand. And the logic flows down like a waterfall but exits as soon as a condition is met.

Upvotes: 1

xificurC
xificurC

Reputation: 1178

There are several ways to factor this:

1) move the nil-check on e-mail higher up your chain. Then your function can do

(let [acct (load-act email)]
  (cond (nil? acct)     "no account"
        (expired? acct) "account expired"
        :else           acct))

2) not all of your tests really need to keep the result. The if-lets can be simplified to

(if (nil? email)
  "email is nil"
  (if-let [acct (load-act email)]
    (if (expired? acct)
      "account expired"
      acct)
    "no account"))

A side-note - clojure's notion of false-ness is nil and false. You're using a nil? check which really checks for nil only, and if-let which checks for false too. There's if-some which only does the nil check.

3) better-cond allows binding inside its cond construct:

(b/cond
  (nil? email) "email is nil"
  :let [acct (load-act email)]
  (nil? acct) "no account"
  (expired? acct) "account expired"
  :else acct)

Upvotes: 2

Related Questions