1
\$\begingroup\$

I have this code inside a post route:

The first one is to alert me when a user registers on my site:

sendgrid.send({
        to:         "my@email.com",
        from:       "myother@email.com",
        subject:    "[ALERT] " + req.body.eventDate,
        html:       "SOME HTML",
    },
    function(err, json) {
        if (err) {
            return console.error(err);
        } 
        else {
            next();
        }
});

The next one is a confirmation email sent to the newly-registered member:

sendgrid.send({
        to:         req.body.email,
        from:       "my@email.com",
        subject:    "[CONFIRM] register" + req.body.eventDate,
        html:       "SOME HTML",
    },
    function(err, json) {
        if (err) {
            return console.error(err);
        } 
        else {
            next();
        }
});

It's working 100%, but this is not a good practice as there is so much duplicatation. Can I DRY this? If so, how?

\$\endgroup\$
  • 1
    \$\begingroup\$ You don't need the else, you have an early return. \$\endgroup\$ – elclanrs Jan 19 '15 at 1:06
2
\$\begingroup\$

Since the code that actually sends the email is the same and only the objects are different, then you can abstract out the sendgrid.send() into a function say sendEmail() that accepts an Object as a parameter since that's what the abstract code does, that's for readability. In the callback function in sendgrid.send() ... you already have a return so anything after that doesn't get executed so don't need the else.

Declare the object for both alert and confirmation first. req.body is repeated in 3 places to declare that too.

Finally, invoke the function sendEmail with twice with each of the objects declared.

var body = req.body,
    alertObject = {
        to: "my@email.com",
        from: "myother@email.com",
        subject: "[ALERT] " + body.eventDate,
        html: "SOME HTML"
    },
    confirmationObject = {
        to: body.email,
        from: "my@email.com",
        subject: "[CONFIRM] register" + body.eventDate,
        html: "SOME HTML"
    };

function sendEmail (emailObject) {
    if (emailObject) {
        sendgrid.send(emailObject, function (err, json) {
            if (err) {
                return console.error(err);
            }
            next();
        });
    }
}

sendEmail(alertObject);
sendEmail(confirmationObject);
\$\endgroup\$
  • \$\begingroup\$ Hi! Welcome to Code Review! Please add an explanation to your code, as a pure-code answer is not very helpful. \$\endgroup\$ – TheCoffeeCup Jan 20 '15 at 21:31
  • \$\begingroup\$ Cheers @MannyMeng, I was being lazy. Been a long day \$\endgroup\$ – dcodesmith Jan 20 '15 at 21:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy