HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Job listings database project in Node.JS

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
nodelistingsdatabaseprojectjob

Problem

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

Solution

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.

Code Snippets

var config = {
    host: process.env.DB_HOST,
    user: process.env.DB_USER,
    // ... etc
}
module.exports = require('knex')({
    client: 'pg',
    connection: {
        host: process.env.DB_HOST,
        user: process.env.DB_USER,
        //...
    }
})
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(/* ... */)
})

Context

StackExchange Code Review Q#156417, answer score: 3

Revisions (0)

No revisions yet.