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.

• holroy: Welcome to Code Review! Could you please tell me if you use an online clojure compiler, or if you run this locally? (I'm interested as I want to test some Clojure, but I don't want to install it on my computer... )
• clojure-learner: Thanks! I'm running everything locally, but if you don't want to install anything, then docker is your friend, I guess

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.

• clojure-learner: Thanks, I agree with all your suggestions. The problem with the code is that it looks somehow too imperative to me, but this is just a feeling.
• Timothy Pratley: gotcha - I don't think you have anything to worry about :)