1
\$\begingroup\$

After cleaning up & some refactoring, one module of my code became like below. However, I'm feeling like there's still some unnecessary redundancy, and it can be improved further and look neater.

But I can't come up with exactly how to do it.

'use strict';

// === showing the execution context ==============

const express = require('express');
const router = express.Router();

const moment = require("moment");
const momentDurationSetup = require("moment-duration-format");
const DB = require('./DBOperator');
const _ = require('lodash');

const Promise = require("bluebird");
const request = require('superagent-bluebird-promise');

const utils = require('../utils');


// ========== main concern area ===============

function realTime(queryFunction, payload, req, res, next) {
    queryFunction((err, results) => {
        if (err) {
            console.log(err);
            next();
        } else {
            res.json({data: _.get(results[0].rows[0], payload, -1)});
        }
    });
}

router.get('/online-buyer-count', utils.mcache(15), (req, res, next) => {
    realTime(DB.onlinebuyerNumQuery, 'onlinebuyerNum', req, res, next);
});

router.get('/offline-buyer-count', utils.mcache(15), (req, res, next) => {
    realTime(DB.offlinebuyerNumQuery, 'offlinebuyerNum', req, res, next);
});

router.get('/online-seller-count', utils.mcache(15), (req, res, next) => {
    realTime(DB.onlinesellerNumQuery, 'onlinesellerNum', req, res, next);
});

router.get('/offline-seller-count', utils.mcache(15), (req, res, next) => {
    realTime(DB.offlinesellerNumQuery, 'offlinesellerNum', req, res, next);
});


/ ===========================

module.exports = router;
\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review! I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to edit and give it a different title if there is something more appropriate. \$\endgroup\$ May 26, 2018 at 2:48
  • \$\begingroup\$ @samOnela that makes sense. Thanks for the edit. \$\endgroup\$
    – kmonsoor
    May 27, 2018 at 14:30

1 Answer 1

1
\$\begingroup\$

Partial application

One way to simplify the code is to eliminate the lambda/anonymous functions using Function.bind() to create Partially applied functions. For instance:

router.get('/online-buyer-count', utils.mcache(15), (req, res, next) => {
    realTime(DB.onlinebuyerNumQuery, 'onlinebuyerNum', req, res, next);
});

can be simplified to

router.get('/online-buyer-count', realTime.bind(null, DB.onlinebuyerNumQuery, 'onlinebuyerNum'));

And similarly for the other three routes, which reduces 12 lines to 4.

looping over the routes

The redundancy could also be simplified using Array.forEach()

['online', 'offline'].forEach(mode => { 
    ['buyer', 'seller'].forEach(role => {
        router.get('/'+mode+'-'+role+'-count', realTime.bind(null, DB[mode+role+'NumQuery'], mode+role+'Num'));
    });
});

Taking that one step further using more partially-applied functions, the callback function of the inner forEach can be pulled out to a separate function:

const addRoute = (mode, role) => router.get('/'+mode+'-'+role+'-count', realTime.bind(null, DB[mode+role+'NumQuery'], mode+role+'Num'));

And then that function can be made into a partial and used within the nested forEach:

['online', 'offline'].forEach(mode => ['buyer', 'seller'].forEach(addRoute.bind(null, mode)));

So that can reduce those 4 lines to 2. It may be desirable to break those up into multiple lines for readability.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks a lot. While the first bind method is more advanced, the second forEach answers my question. It looks more approachable. \$\endgroup\$
    – kmonsoor
    May 30, 2018 at 19:34

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.