Sending mail using nodemailer in Clojure

Question

I'm very new to Clojure/ClojureScript and have a question regarding a "proper" way to express something.

This code sends a mail using nodemailer on nodejs:

(defn send-mail [some-id]
  (try
    (let [clj-props (:mail (config/config-file)) ; get the mail server cofiguration
          mail-address (mail-adress-from-props clj-props) ; get the mail address
          mail (clj->js (create-mail mail-address some-id)) ; create a mail to send
          transporter (.createTransport nodemailer (clj->js clj-props))] ; create a transporter using nodemailer
      (.sendMail transporter mail on-mail-sent)) ; send mail using nodemailer
    (catch js/Error e
      (logging/error (.toString e)))))

It is working, but looks completely out of place in ClojureScript. I woud like for a more idiomatic way to do this, so any feedback is welcome.

Answer 1

In my opinion the code looks great. What in particular looks out of place? When dealing with interop, there is no need to get hung up style, you are going to have to match the model exposed to you. I don't think there is any more proper way than as you have done.

Just as a matter of style I would make these suggestions based on my personal tastes:

1. Lean toward functions that take arguments over looking things up from the environment. Consider passing the result of create-mail to send-mail as an argument. Consider passing on-mail-sent to send-mail as an argument. Your functions will be more flexible this way.

2. Consider putting the try catch log around the call to send-mail, instead of inside it. Functions tend to be more reusable when they allow the caller to handle errors either by returning a result or possibly throwing.

3. Don't bother with comments that repeat the code.

4. nodemailer appears to be a global; should this be called like (js/nodemailer.createTransport)? Or is it a def?

5. clj-props doesn't convey meaning. Consider mail-config?

6. What does mail-adress-from-props do? is it just a keyword lookup? If so consider just using the direct lookup instead.