2
\$\begingroup\$

I want to create variable paging on Node.js with MongoDB. I use MongoJS on client side create a request:

api\logs?&limit=val1&page=val2&sort=val3&order=val4&field1=val5&field2=val6

where

  • limit, page - paging sort
  • order, sort - sorting (sort - field. "title","age" & etc)
  • field1, filed2 ... - option for filter

are all parameters optionally.

What are the disadvantages of the method of processing the request and retrieving data? The planned volume of the collection of up to 100,000 records, fields, indexes are created.

var listSort = ['time','request'];
var defSort = listSort[0];
var listOrder = ['-1','1'];
var defOrder = listOrder[0];

router.get('/', function(req, res){
   var limit = (req.query.limit === undefined) ? 25 : req.query.limit;
   var page = (req.query.page === undefined) ? 1 : req.query.page;

   var sort =  listSort.indexOf(req.query.sort) == -1 ? defSort :  req.query.sort;
   var order = listOrder.indexOf(req.query.order) == -1 ? defOrder : req.query.order;
   var querySort = {};
   querySort[sort] = parseInt(order);

   var timeBegin = isNaN(Number(req.query.timeBegin)) ? undefined : Number(req.query.timeBegin);
   var timeEnd = isNaN(Number(req.query.timeEnd))  ? undefined : Number(req.query.timeEnd);
   var request = req.query.request;
   var queryFind = {};

   if(timeBegin || timeEnd){
       queryFind['time']={};
       if(timeBegin)
           queryFind['time']['$gte']=timeBegin;
       if(timeEnd)
           queryFind['time']['$lte']=timeEnd;
   }

   if(request){
       queryFind['request']=  new RegExp(request, 'i');
   }

   Log.find(queryFind).sort(querySort)
   .limit(limit).skip(limit*(page-1), function(err, logItems) {
       if (!err) res.json(logItems);
       else res.send({error: err});
   })});
  • limit - the number of entries per page
  • page - he current page number
  • listSort - he list consists of a list of fields available for sorting
  • defSort - sort by default
  • listOrder - the list of sort order
  • defOrder - default order

Filter for 'time'. Start and end. Perhaps no one of significance. Filter 'request'. This text field, search for possible inclusion content.

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

I don't have any good feed back on the technical aspects of your api calls, which look fine to me. Being that this is code review here is my code review.

I would set a variable to your req.query parameter, which will make the code look cleaner.

Your ternaries can be shortened to:

req.query.limit || 25;

An undefined variable throws an Error, an undefined object property is falsy.

Always use === over ==

Parse Int must always have a radix, otherwise we cannot guarantee consistent performance. (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt)

parseInt(order, 10) //Assuming this is sent in decimal

Always return res.send while you are using in fine in this context, adding the return is a safety measure for the future, all you need is someone to change the logic, not properly test error cases and you have a double callback that sometimes can be hard to debug.

So that results in:

var listSort = ['time','request'];
var defSort = listSort[0];
var listOrder = ['-1','1'];
var defOrder = listOrder[0];

router.get('/', function(req, res){
   var rQuery = req.query || {};
   var limit = rQuery.limit || 25;
   var page = rQuery.page || 1;

   var sort =  listSort.indexOf(rQuery.sort) === -1 ? defSort :  rQuery.sort;
   var order = listOrder.indexOf(rQuery.order) === -1 ? defOrder : rQuery.order;
   var querySort = {};
   querySort.sort = parseInt(order, 10);

   var timeBegin = isNaN(Number(rQuery.timeBegin)) ? undefined : Number(rQuery.timeBegin);
   var timeEnd = isNaN(Number(rQuery.timeEnd))  ? undefined : Number(rQuery.timeEnd);
   var request = rQuery.request;
   var queryFind = {};

   if(timeBegin || timeEnd){
       queryFind.time={};
       if(timeBegin)
           queryFind.time['$gte']=timeBegin;
       if(timeEnd)
           queryFind.time['$lte']=timeEnd;
   }

   if(request){
       queryFind.request =  new RegExp(request, 'i');
   }

   Log.find(queryFind).sort(querySort)
   .limit(limit).skip(limit*(page-1), function(err, logItems) {
       if (!err) {
         return res.json(logItems);
       } else {
         return res.send({error: err});
       }
   })});
\$\endgroup\$

Your Answer

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

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