9
\$\begingroup\$

Build a project which can allow user to post and search for job listings, I have written some code and divided into different modules.

My motivation is to understand how much over-engineering is needed or not for a system like this which is basically a CRUD system.

db.js

var pg = require('pg');

var config = {
  host: 'localhost',
  user: 'v',
  password: 'a',
  database: 'j',
};

var pool = new pg.Pool(config);

pool.connect(function(err, client, done) {
  if(err) {
    return console.error('error fetching client from pool', err);
  }
  client.query('SELECT $1::int AS number', ['1'], function(err, result) {
    done(err);

    if(err) {
      return console.error('error running query', err);
    }
    console.log(result.rows[0].number);
    //output: 1
  });
});

pool.on('error', function (err, client) {
  // if an error is encountered by a client while it sits idle in the pool
  // the pool itself will emit an error event with both the error and
  // the client which emitted the original error
  // this is a rare occurrence but can happen if there is a network partition
  // between your application and the database, the database restarts, etc.
  // and so you might want to handle it and at least log it out
  console.error('idle client error', err.message, err.stack)
});

module.exports = pool;

index.js

'use strict';

var express = require('express');
var Promise = require('promise');
var router = express.Router();
var app = express();

var pool = require('./db.js')();
var User = require('./models');

var PORT = 3000;

app.use(function (req, res, next) {
  pool.connect(function(error, client, done) {
    // Handle connection errors
    if (error) {
      done(error);
      console.log(error.message);
      return res.status(500)
          .json({success: false, data: error});
    }
    req.client = client;
    req.done = done;
    next();
  });
});
**index.js**

'use strict';

var express = require('express');
var Promise = require('promise');
var router = express.Router();
var app = express();

var pool = require('./db.js')();
var User = require('./models');

var PORT = 3000;

app.use(function (req, res, next) {
  pool.connect(function(error, client, done) {
    // Handle connection errors
    if (error) {
      done(error);
      console.log(error.message);
      return res.status(500)
          .json({success: false, data: error});
    }
    req.client = client;
    req.done = done;
    next();
  });
});

router.get('/topActiveUsers', (req, res) => {
  topActiveUsers(req, res);
});

router.get('/users', (req, res) => {
  userInfo(req, res);
});

app.use(router);

app.get('*', function (req, res) {
  res.status(400).send('Invalid route');
});

app.listen(PORT, function () {
  console.log('App listening on port ' + PORT);
});

var topActiveUsers = function topActiveUsers(req, res) {
  var ENTRIES_PER_PAGE = 3;
  var startIndex = 0;
  var total = 0;
  req.query.page = +req.query.page || 0;

  var pageNum = req.query.page > 0 ? req.query.page : 0;
  if (pageNum > 0) {
    startIndex = ENTRIES_PER_PAGE * (pageNum - 1);
  }
  total = ENTRIES_PER_PAGE * (pageNum + 1);

  User.topActiveUsers(req)
      .then(function fullfilled(users) {
        if (users.length < startIndex) {
          throw new Error('Invalid pagination offset');
        }
        if (users.length > total) {
          users = users.slice(startIndex, startIndex + ENTRIES_PER_PAGE);
        } else {
          users = users.splice(startIndex);
        }
        return Promise.all(users.map(function (user) {
          return user.applicationListings(req);
        }));
      })
      .then(function fullfilled(users) {
        var result = users.map(function (user) {
          return {
            id: user.id,
            name: user.name,
            count: user._appliedListings.length,
            createdAt: user.createdAt,
            listings: user._appliedListings
          };
        });
        res.json(result);
      })
      .catch(function rejected(error) {
        console.log(error.message);
        throw error;
      })
      .finally(function () {
        res.end();
      });
};

var userInfo = function userInfo(req, res) {
  User.getById(req)
      // run companies/listings/applications in "parallel"
      .then(function fullfilled(user) {
        return Promise.all([
          user.id,
          user.name,
          user.createdAt,
          user.companies(req),
          user.listings(req),
          user.applications(req)
        ]);
      })
      .then(function fullfilled([
            id, name, createdAt, companies, listings, applications]) {
        res.json({
          id: id,
          name: name,
          createdAt: createdAt,
          companies: companies,
          listings: listings,
          applications: applications
        });
      })
      .catch(function rejected(error) {
        console.log('error', error.message);
        throw error;
      })
      .finally(function () {
        res.end();
      });
};

models/index.js

var Promise = require('promise');

module.exports = User;

/**
 * User m2m Company
 * User o2m Listing
 * User m2m applications
 */
function User(opt_data) {
  var data = opt_data || {};

  this.id = data['id'] || null;
  this.name = data['name'] || '';
  this.createdAt = data['created_at'] || new Date();
  this._companies = [];
  this._listings = [];
  this._applications = [];
  this._appliedListings = [];
}
User._RESOURCE_LIMIT = 5;
var UserProto = User.prototype;

User.topActiveUsers = function topActiveUsers(req) {
  var queryString = "select * from users u inner join "+
      "(select user_id, count(id) cnt from applications "+
      "where id in (select id from applications where "+
      "created_at > current_date - interval '1 week') "+
      "group by user_id) a on u.id = a.user_id order by a.cnt desc";
  return queryPromise(req, queryString)
      .then(function fullfilled(result) {
        return result.rows.map(function(row) {
          return new User(row);
        });
      });
};

User.getById = function getById(req) {
  var queryString = 'select * from users where id = $1::int';
  return queryPromise(req, queryString, [req.query.id])
      .then(function fullfilled(result) {
        return new User(result.rows[0]);
      });
};

UserProto.companies = function companies(req) {
  var queryString = 'select c.id, c.name, t.contact_user '+
      'from companies c, teams t '+
      'where t.user_id = $1::int and t.company_id = c.id '+
      'limit $2::int';
  return queryPromise(req, queryString, [this.id, User._RESOURCE_LIMIT])
    .then(function fullfilled(result) {
      return result.rows.map(function (data) {
        return new Company(data);
      });
    });
};

UserProto.listings = function listings(req) {
  var queryString = 'select * from listings '+
      'where created_by = $1::int '+
      'limit $2::int';
  return queryPromise(req, queryString, [this.id, User._RESOURCE_LIMIT])
    .then(function fullfilled(result) {
      return result.rows.map(function (data) {
        return new Listing(data);
      });
    });
};

UserProto.applicationListings = function applications(req) {
  var queryString = "select * from listings l inner join "+
      "(select listing_id, user_id, created_at from applications) a "+
      "on a.listing_id = l.id "+
      "where a.user_id = $1::int order by a.created_at desc limit 3";
  var self = this;
  return queryPromise(req, queryString, [this.id])
      .then(function fullfilled(result) {
        self._appliedListings = result.rows.map(function (data) {
          return new Listing(data);
        });
        return self;
      });
};

UserProto.applications = function applications(req) {
  var queryString = 'select a.id as app_id, a.created_at, a.cover_letter, '+
    'l.id as list_id, l.name, l.description '+
    'from applications a, listings l '+
    'where a.user_id = $1::int and a.listing_id = l.id '+
    'limit $2::int';
  return queryPromise(req, queryString, [this.id, User._RESOURCE_LIMIT])
      .then(function fullfilled(result) {
        return result.rows.map(function (data) {
          return new Application(data);
        });
      });
};

function Company(opt_data) {
  var data = opt_data || {};
  this.id = data['id'] || null;
  this.createdAt = data['created_at'] || new Date();
  this.name = data['name'] || '';
  this.isContact = false;
}

function Listing(opt_data) {
  var data = opt_data || {};
  this.id = data['id'] || null;
  this.createdAt = data['created_at'] || new Date();
  this.name = data['name'] || '';
  this.description = data['description'] || '';
}

function Application(opt_data) {
  var data = opt_data || {};
  this.id = data['id'] || null;
  this.createdAt = data['created_at'] || new Date();
  this.listing = data['listing'] || null;
  this.coverLetter = data['cover_letter'] || '';
}

function queryPromise(req, queryString, queryParams, debug) {
  if (debug) {
    console.log(queryString, queryParams);
    req.client.connection.on('message', function(msg) {
      console.log(msg)
    });
  }
  return new Promise(function _promise(resolve, reject) {
    req.client.query(
        queryString,
        queryParams || [],
        function result(error, result) {
      req.done(error);

      if (error) {
        console.log('error ' + error.message);
        return reject(error);
      }
      resolve(result);
    });
  });
};

db.js

var pg = require('pg');

module.exports = function() {
  var config = {
    port: 5432,
    max: 10,
    idleTimeoutMillis: 30000
  };
  switch (process.env.NODE_ENV) {
    case 'development':
      config.host = 'localhost';
      config.user = 'xxxx';
      config.password = 'xxxx';
      config.database = 'xxxx';
      break;
    case 'production':
      config.user = 'xxxx';
      config.database = 'xxxx';
      config.password = 'xxxx';
      config.host = 'xxxx'
      break;
    default:
      throw new Error('Invalid enviroment');
  }

  var pool = new pg.Pool(config);

  pool.connect(function(err, client, done) {
    if(err) {
      return console.error('error fetching client from pool', err);
    }
    client.query('SELECT $1::int AS number', ['1'], function(err, result) {
      done(err);

      if(err) {
        return console.error('error running query', err);
      }
      console.log(result.rows[0].number);
      //output: 1
    });
  });

  pool.on('error', function (err, client) {
    // if an error is encountered by a client while it sits idle in the pool
    // the pool itself will emit an error event with both the error and
    // the client which emitted the original error
    // this is a rare occurrence but can happen if there is a network partition
    // between your application and the database, the database restarts, etc.
    // and so you might want to handle it and at least log it out
    console.error('idle client error', err.message, err.stack)
  });

  return pool;
};

tables.sql

create table users (
    id serial primary key,
    created_at timestamp default current_timestamp,
    name character varying(64)
);

create table companies (
    id serial primary key,
    created_at timestamp default current_timestamp,
    name character varying(64)
);

create table teams (
    id serial primary key,
    company_id integer references companies (id),
    user_id integer references users (id),
    contact_user boolean default false
);

create table listings (
    id serial primary key,
    created_at timestamp default current_timestamp,
    created_by integer references users (id),
    name character varying(64),
    description text
);

create table applications (
    id serial primary key,
    created_at timestamp default current_timestamp,
    user_id integer references users (id),
    listing_id integer references listings (id),
    cover_letter text
);
router.get('/topActiveUsers', (req, res) => {
  topActiveUsers(req, res);
});

router.get('/users', (req, res) => {
  userInfo(req, res);
});

app.use(router);

app.get('*', function (req, res) {
  res.status(400).send('Invalid route');
});

app.listen(PORT, function () {
  console.log('App listening on port ' + PORT);
});

var topActiveUsers = function topActiveUsers(req, res) {
  var ENTRIES_PER_PAGE = 3;
  var startIndex = 0;
  var total = 0;
  req.query.page = +req.query.page || 0;

  var pageNum = req.query.page > 0 ? req.query.page : 0;
  if (pageNum > 0) {
    startIndex = ENTRIES_PER_PAGE * (pageNum - 1);
  }
  total = ENTRIES_PER_PAGE * (pageNum + 1);

  User.topActiveUsers(req)
      .then(function fullfilled(users) {
        if (users.length < startIndex) {
          throw new Error('Invalid pagination offset');
        }
        if (users.length > total) {
          users = users.slice(startIndex, startIndex + ENTRIES_PER_PAGE);
        } else {
          users = users.splice(startIndex);
        }
        return Promise.all(users.map(function (user) {
          return user.applicationListings(req);
        }));
      })
      .then(function fullfilled(users) {
        var result = users.map(function (user) {
          return {
            id: user.id,
            name: user.name,
            count: user._appliedListings.length,
            createdAt: user.createdAt,
            listings: user._appliedListings
          };
        });
        res.json(result);
      })
      .catch(function rejected(error) {
        console.log(error.message);
        throw error;
      })
      .finally(function () {
        res.end();
      });
};

var userInfo = function userInfo(req, res) {
  User.getById(req)
      // run companies/listings/applications in "parallel"
      .then(function fullfilled(user) {
        return Promise.all([
          user.id,
          user.name,
          user.createdAt,
          user.companies(req),
          user.listings(req),
          user.applications(req)
        ]);
      })
      .then(function fullfilled([
            id, name, createdAt, companies, listings, applications]) {
        res.json({
          id: id,
          name: name,
          createdAt: createdAt,
          companies: companies,
          listings: listings,
          applications: applications
        });
      })
      .catch(function rejected(error) {
        console.log('error', error.message);
        throw error;
      })
      .finally(function () {
        res.end();
      });
};

data.sql

insert into users (id, created_at, name) values
    (1, '2015-01-13 15:30', 'Mark'),
    (2, '2015-01-13 15:30', 'John'),
    (3, '2016-01-01 10:30', 'Melinda'),
    (4, '2016-01-17 23:30', 'Carl'),
    (5, '2016-02-02 16:30', 'Tim'),
    (6, '2016-02-02 16:30', 'Jessica')
;

insert into companies (id, created_at, name) values
    (1, '2015-01-13 15:00', 'Facewall'),
    (2, '2015-01-17 15:00', 'Carl & Co')
;

insert into teams (company_id, user_id, contact_user) values
    (1, 1, TRUE),
    (2, 3, FALSE),
    (2, 4, TRUE)
;

insert into listings (id, created_at, created_by, name, description) values
    (1, '2015-01-15 11:00', 1, 'Join us conquering the world!', 'This is your best chance to be on the right side of the equation...')
;

insert into applications (created_at, user_id, listing_id, cover_letter) values
    ('2015-01-16 12:00', 2, 1, 'Hello, ...')
;

Notes:

  1. I need to apply dependency injection from start. I don't have enough experience in NodeJS but I have tried one approach here. I would like to know if there is a better approach.
  2. Is the code flexible and maintainable? I see some repetitive logic. How can I refactor it?
  3. Can my code handle thousands of requests? If not, what changes do I need to make?
  4. What if I submitted this code as part of an interview process, and got rejected. I would really love to know what I did wrong.
\$\endgroup\$
  • 3
    \$\begingroup\$ obvious "change" is to remove .then(function (user) { return user; }) - \$\endgroup\$ – Jaromanda X Feb 28 '17 at 4:40
  • \$\begingroup\$ @JaromandaX can you give me some important tips from architecture point of view? \$\endgroup\$ – CodeYogi Feb 28 '17 at 4:55
  • \$\begingroup\$ To be honest, I don't know what you mean by that - I can only make suggestion about Note 2 - which you could probably figure out from the answer on your question in stack overflow \$\endgroup\$ – Jaromanda X Feb 28 '17 at 5:18
  • \$\begingroup\$ Not sure if this code helps in at least removing the repetitive code \$\endgroup\$ – Jaromanda X Feb 28 '17 at 5:21
  • \$\begingroup\$ why is there two tables.sql and two models/index.js and two db.js in updated code which one to use ??. Please can you remove redundant code to make question clear \$\endgroup\$ – user93 Mar 3 '17 at 5:10
2
\$\begingroup\$

Notes about db

Try not to keep your connection variables inside of your code. Best practice is to move them to environment variables.

var config = {
    host: process.env.DB_HOST,
    user: process.env.DB_USER,
    // ... etc
}

This eliminates the need for your switch (process.env.NODE_ENV). Also, this gives some level of security, because you do not store your credentials in a repository (imagine this being a public repository and everyone knows your access keys lol).

You can replace your db module with ready-made one called knex. It basically manages your pool for you and gives access to query builder. Do not hesitate to use npm modules. There are a lot of great tools there that can eliminate some headache in development.

Your db module could look like this with knex

module.exports = require('knex')({
    client: 'pg',
    connection: {
        host: process.env.DB_HOST,
        user: process.env.DB_USER,
        //...
    }
})

And your UserProto.listings query

const builder = knex('listings')
    .where('created_by', this.id)
    .limit(User._RESOURCE_LIMIT)
return builder.then(function (rows) {
    return rows.map(/* ... */)
})

// -- or you can always use raw queries if needed.

const builder = knex.raw('YOUR RAW QUERY HERE')
return builder.then(function (res) {
    return res.rows.map(/* ... */)
})

And as I can see you are using ES6 features in your code, so it's better to replace your var with let and const (preferably). There are a lot of articles over the internet about advantages of them, so I won't duplicate them here.

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