2
\$\begingroup\$

Is this approach considered good practice, or should I create separate router.delete functions for all my routes? Please, explain why.

router.delete('/delete-:object', function(req, res) {
   var query;
   var id = req.body.id;

   switch (req.params.object) {

       case 'news' :
           query = queries['news_delete'];
           break;

       case 'member' :
           query = queries['member_delete'];
           break;

       case 'account' :
           query = queries['account_delete'];
           break;

       default :
           res.sendStatus(404);
           return;
   }

   connection.query(query, id);
   res.sendStatus(200);
});
\$\endgroup\$
1
\$\begingroup\$

If most of your code is the same among all the different delete URLs you are processing, then it makes perfect sense to share one route that branches based on the minor difference between them. Since that is what it looks like you have, then the way you have it is a logical way to do it.

You could simplify your current technique a bit:

router.delete('/delete-:object', function(req, res) {
   var query, item = req.params.object;

   switch (item) {
       case 'news' :
       case 'member' :
       case 'account' :
           query = queries[item + '_delete'];
           break;
       default :
           res.sendStatus(404);
           return;
   }

   connection.query(query, req.body.id);
   res.sendStatus(200);
});

Or, you could even remove the switch statement in favor of a lookup table:

router.delete('/delete-:object', function(req, res) {
   var query, item = req.params.object;
   var deleteObjects = {news: true, member: true, account: true};

   if (deleteObjects.hasOwnProperty(item)) {
       connection.query(queries[item + '_delete'], req.body.id);
       res.sendStatus(200);
   } else {
       res.sendStatus(404);
   }
});

You could also move the common code to a function with an argument and do it this way:

function processDelete(q) {
    return function(req, res) {
        connection.query(queries[q], req.body.id);
        res.sendStatus(200);        
    }
}

router.delete('/delete-news', processDelete("news-delete"));
router.delete('/delete-member', processDelete("member-delete"));
router.delete('/delete-account', processDelete("account-delete"));

I'd say that there is no "right" or "wrong" argument about which of these approaches is correct. The last approach is perhaps a little quicker to read exactly what it's doing since the routes are all laid out there for you and the handler function is all right there and there's no mixing of the logic of the two, but that's as much a personal opinion as anything else, not really a firm argument one way or the other.

\$\endgroup\$

Your Answer

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

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