1
\$\begingroup\$

I have an app where users at some point select the amount of tasks they have completed. If they have only completed 3 or less then 3 tasks then I send an email to the system admin to inform them.

But at the same time as the email is being sent the I want the data to be saved into the database. To achieve this I implemented a callback (or what I think is a callback.)

Is the below implementation okay?

if(req.body.sessionNo == 3){
                newDoc.sessionThreeMileStones = req.body.sessionThreeMileStones;
                // CALLBACK!!!
                checkMileStones(req.body,req.body.sessionThreeMileStones, sendMail)
    }

//SAVING THE DATA INTO MONGODB
newDoc.save(function(err, data) {
                if (err) {
                    sendJsonResponse(res, 400, err)
                }
                else {
                    incrementSessionTypesTotal(req, res);
                    sendJsonResponse(res, 200, data);
                }
            });

  // PASSING THE CALLBACK
  function checkMileStones(body,milestones,callback){
    if(milestones.length <= 3){
        //CALLING THE CALLBACK
        callback(body,milestones)
    } else {
        console.log("Not required");
    }
};

 // ACTUAL CALLBACK FUNCTION
 function sendMail(data,milestones){
    var data = {
                    from: 'App Team',
                    to: '',
                    subject: 'Milestones Alert - Session ' + data.sessionNo,
                    text: 'Hi ' + '\n\n' + data.fullName + ' from ' + data.company + ' has submitted the data for 3rd session with ' + data.clientName +'.\n' + 'They have only completed ' + milestones.length + ' milestones so far.'
                        };

                        mailgun.messages().send(data, function(error, body) {
                            console.log("sending email " + body)
                       });
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Your implementation will work but in my opinion you have made your code complicated for no reason. You are violating the "Single Responsibility" principle here by making your "checkMilestones" method do two things

  1. Check if milestone is reached
  2. Send email

You could have simplified this as follows:

(req.body.sessionThreeMileStones.length <=3) ? sendMail() : doNothing();

This way your code is more readable and also easy to debug.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.