Web service to add a phone number with SMS validation

Question

I'm new to Clojure. How can I make this more idiomatic Clojure code? Among other things, there are three (do) blocks and I don't think using (def) as many times as I do is a recommended idea.

(defn add-phone
 "Add a phone number to a given service. There are a few steps we have to take:
 - Validate the service name and token
 - Validate the phone number 
 - Send a text message to the number
 - Store the number in the database
 Arguments:
 request - an authorized HTTP request
 Returns:
 201 Resource Created - phone number added to a service
 400 Bad Request - invalid phone number or bad request"
 [request auth-type]
 (do
 (println "Url requested is " (:request-method request) " " (:uri request))
 (println request)
 (def auth-header ((:headers request) "authorization"))
 (def auth-map (utils/get-auth-header auth-type auth-header))
 (if (model/invalid-user-and-pass auth-map auth-type)
 {:status 401,
 :headers {"WWW-Authenticate" "Basic realm=\"smsauth.herokuapp.com\""}}
 (do
 (def raw-phone-no (:phone_number (:params request)))
 (def service-name (:service-name auth-map))
 (if raw-phone-no
 (do 
 (def valid-phone-no (utils/parse-phone-no raw-phone-no))
 ; XXX get the service id some other way.
 (def service-map (model/service-in-db service-name))
 (def service-id (service-map :id))
 ; XXX check that service doesn't already have phone number
 (model/store-phone-no :numbers service-id valid-phone-no)
 (twilio/send-sms-verify twilio/sms-url valid-phone-no service-name)
 ; XXX jsonify this, send more information, document it.
 {:status 201,
 :body (json/json-str {
 :service_name service-name,
 :phone_number valid-phone-no,
 :status "active",
 :uri (:uri request)
 ; should have some kind of unique id for the phone number
 ; here.
 ; also return the date
 })})
 {:status 400,
 :body (json/json-str {:error "Please include a phone_number."})})))))

Answer 1

There are several errors/bad styles here:

• First do isn't necessary, as body of function can contain several expressions
• instead of def inside function it's better to use let that allows to "define" several variables in one step, and they will visible only inside this let block. So 2nd & 3rd do could be replaced with corresponding let
• if variable is used only once, then you can include function call directly into map

So your code will look like:

(defn add-phone
 [request auth-type]
 (println "Url requested is " (:request-method request) " " (:uri request))
 (println request)
 (let [auth-header ((:headers request) "authorization")
 auth-map (utils/get-auth-header auth-type auth-header)]
 (if (model/invalid-user-and-pass auth-map auth-type)
 {:status 401,
 :headers {"WWW-Authenticate" "Basic realm=\"smsauth.herokuapp.com\""}}
 (let [raw-phone-no (:phone_number (:params request))
 service-name (:service-name auth-map)]
 (if raw-phone-no
 (let [valid-phone-no (utils/parse-phone-no raw-phone-no) ; XXX get the service id some other way.
 service-map (model/service-in-db service-name)
 service-id (service-map :id) ; XXX check that service doesn't already have phone number
 ]
 (model/store-phone-no :numbers service-id valid-phone-no)
 (twilio/send-sms-verify twilio/sms-url valid-phone-no service-name)
 ; XXX jsonify this, send more information, document it.
 {:status 201,
 :body (json/json-str {
 :service_name service-name,
 :phone_number valid-phone-no,
 :status "active",
 :uri (:uri request)
 ; should have some kind of unique id for the phone number
 ; here.
 ; also return the date
 })})
 {:status 400,
 :body (json/json-str {:error "Please include a phone_number."})})))))

P.S. it's also good idea to check value of service-map - if it will nil, then you'll get NPE. So instead of (service-map :id) you can write (:id service-map) that will correctly work if service-map will nil...