6
\$\begingroup\$

I'm simple looking for what the general preference is between the below options:

  1. anon function directly to variable

    middleware.customer = function(req, res, next){
        req.customer = function(){
            var customer = false;
            if(req.method == "POST") customer = req.body;
            if(req.method == "GET") customer = req.query;
            return customer;            
        }();
        return next();
    }
    
  2. raw build variable up

    middleware.customer = function(req, res, next){
        req.customer = false;
        if(req.method == "POST") req.customer = req.body;
        if(req.method == "GET") req.customer = req.query;
        return next();
    }
    
  3. complete separation

    var customer = function(req){
        var customer = false;
        if(req.method == "POST") customer = req.body;
        if(req.method == "GET") customer = req.query;
        return customer;            
    }
    
    middleware.customer = function(req, res, next){
        req.customer = customer(req);
        return next();
    }
    
\$\endgroup\$
  • 2
    \$\begingroup\$ Second version looks best IMO. \$\endgroup\$ – elclanrs Jan 18 '14 at 22:06
  • \$\begingroup\$ That's where I'm at @elclanrs \$\endgroup\$ – ThomasReggi Jan 18 '14 at 22:17
8
\$\begingroup\$

I think B is the best of the three, but here's another alternative using a simple lookup:

middleware.customer = function(req, res, next){
  var methods = {POST: req.body, GET: req.query};
  req.customer = methods[req.method] || false;
  return next();
}
\$\endgroup\$
5
\$\begingroup\$

The second approach is best because it is the simplest. I would use a ternary conditional to make it clearer that the intention is to assign something to req.customer.

middleware.customer = function(req, res, next){
    req.customer = (req.method == "POST") ? req.body :
                   (req.method == "GET") ? req.query : null;
    return next();
};

I would also put an explicit semicolon to terminate the function definition statement.

\$\endgroup\$
3
\$\begingroup\$

I keep thinking that you will need this way more than just for customer, and that you should have a function for this so that you keep it DRY.

The tricky thing is naming, how do you call the action of taking either the entire query or the entire body? When would you do this ( only in REST I guess, but even then, what use is POSTing when the entire body only has a customer ID ?, maybe to do an action that only requires the customer ID I guess)

//request.query / request.body represent 1 value, return that value
function getRequestValue( req )
{
  return (req.method == "GET")   ? req.query  :
         (req.method == "POST" ) ? req.body   : false;  
}

middleware.customer = function(req, res, next)
{
  req.customer = getRequestValue( req );
  return next();
}

I borrowed some of @200_success here, except that I check for GET first since that should be more common and I return false since that is what you set req.customer initially.

\$\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.