1

I am creating a small project in node, i have routes of two tables, drums and pianos in same file named routes.js in two different function called drums() for drums and pianos() for pianos. i have exported both functions and both of them return router. Router is return value of express.Route() class.

I have used these two functions in index.js file by importing, i am accessing route by following code

app.use('/pianos', routes.pianos());
app.use('/drums', routes.drums());

But i am getting response from only one function, if i keep

app.use('/pianos', routes.pianos());

at first then i get list of pianos with url localhost/pianos and localhost/drums as well and if i keep

app.use('/drums', routes.drums());

at first then i get list of drums with url localhost/drums and localhost/pianos as well. What is mistake here?

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

function drums(){
    drums = [
        {id:1, name:'Bongo drum'},
        {id:2, name:'Bass drum'},
        {id:3, name:'Ashiko'},
        {id:4, name:'Basler drum'},
        {id:5, name:'Cajón'}
    ];

    router.get('/', (req, res)=>{
        res.send(drums);
    });
    router.get('/:id', (req, res) =>{
        const drum = drums.find( c=> c.id === parseInt(req.params.id));

        if(!drum){
            return res.status(404).send("Error: Drum is not available");
        }
        res.send(drum.name);
    });
    router.post('/', (req, res)=>{
        const {error} = validator(req.body);
        if(error) return res.status(400).send('Eror 400: Bad request', error.details[0].message); 

        const drum = {id:drums.length+1, name:req.body.name};

        drums.push(drum);
        res.send(drum);        
        }   
    );
    router.put('/:id', (req, res)=>{
        const drum = drums.find(c=> c.id === parseInt(req.params.id));
        if(!drum) return res.status(404).send('Error: Drum is not available');

        const {error} = validator(req.body);
        if(error) return res.status(400).send('Error 400: Bad request', error.details[0].message);

        drum.name = req.body.name;
        res.send(drum);
    });
    return router;
}

function pianos(){
    const pianos = [
        {id:1, name:'Yamaha U1'},
        {id:2, name:'Roland RD-2000'},
        {id:3, name:'Nord Piano 4'},
        {id:4, name:'Yamaha NU1X'},
        {id:5, name:' Dexibell Vivo S7'}
    ];

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

    router.get('/:id', (req, res)=>{
        const piano = pioanos.find(c=> c.id === parseInt(req.params.id));
        if(!piano) return res.status(404).send('Error:The piano is not available');

        res.send(piano);
    });

    router.post('/', (req, res)=>{
        const {error} = validator(req.body);
        if(error) return res.status(400).send('Error-Bad request', error.details[0].message);

        const piano = {id:pianos.length+1, name:req.body.name};

        pianos.push(piano);
        res.send(piano);
    });

    router.put('/:id', (res, req)=>{
        const piano = pioanos.find(c=> c.id === parseInt(req.params.id));
        if(!piano) return res.status(404).send('Error: Piano is the available');

        const {error} = validator(req.body);
        if(error) return res.status(400).send('Error:Bad request', error.details[0].message);

        piano.name = req.body.name;
        res.send(piano);
    });
    return router;
}

function validator(instrument){
    const schema={
        name:joi.string().min(5).required()
    };
    return joi.validate(instrument, schema);

}



module.exports.drums = drums;
module.exports.pianos = pianos;

MY index.js file is like this:

    const mongoose = require('mongoose');
    const express = require('express');
    const app = express();
    const debug = require('debug')('pm:index');
    const routes = require('./routes/routes');    

    mongoose.connect('mongodb:localhost/planetmusic')
        .then(()=> debug('Connected to database'))
        .catch(err => debug('Error!!Could not connect to database', err.message));

    app.use(express.json());
    app.use(express.urlencoded({extended: true}));
    app.use(express.static('public'));


    app.use('/drums', routes.drums());
    app.use('/pianos', routes.pianos());


    const port = process.env.port || 5000;
    app.listen(port, ()=> console.log(`listening at port: ${port}`));

If there are other better solution to manage my all routes then please help me.

Alex Wayne
  • 178,991
  • 47
  • 309
  • 337
Jhapali
  • 11
  • 3
  • I need help, should i use separate module for different routes ? – Jhapali Jan 10 '20 at 00:02
  • most generic routes should go after more specific ones, i.e.: '/:id' first, '/' last https://stackoverflow.com/questions/32603818/order-of-router-precedence-in-express-js – Vadim Aidlin Jan 10 '20 at 00:46
  • On a quick glance, I wonder why you have duplicate route definitions? For example, you have a route definition "/" twice in your entire app. Once in drums(), once in piano(). Barring anything else, I would think you should have just one occurrence of route definition for "/" (easiest to manage) and within that you'd differentiate drums vs piano's logic. – Kalnode Jan 10 '20 at 00:50
  • Can i have routes for two different path inside two functions in same module or not? if i can then i need some help otherwise i will use different file for routes of different path. What should i do? – Jhapali Jan 10 '20 at 01:03

1 Answers1

0

taken that we should separate concerns, I would recommend split everything into files, it is easier to maintain and to add features in the future as everything is in it's own place...

in a NodeJs project, it is normal to encounter, in examples, some folder rearrangement, and I would suggest this:

.
├── package.json
└── server/
    ├── server.js
    └── routes/
        ├── drums.js
        ├── index.js
        └── piano.js

here is a very simple example, so you can understand how ExpressJs routes work when spread out upon multiple files:

server.js content

const express = require('express');

const routes = require('./routes');
const app = express();

const PORT = 5001;

app.use('/drums', routes.drums);
app.use('/pianos', routes.pianos);
app.use('/', (req, res) => { res.send({ action: 'default get' }); });

app.listen(PORT, () => { 
    require('./utilities/api-table')(app._router.stack);
    console.log(`ready on http://localhost:${PORT}`);
});

server/routes/index.js content

const fs = require("fs");
const path = require("path");

const routing = {};

fs.readdirSync(__dirname) // read all files in this directory
  .filter(
    file =>
      // only read .js files, but not the index.js (this file)
      file.indexOf(".") !== 0 && file !== "index.js" && file.slice(-3) === ".js"
  )
  .forEach(file => {
    const filename = file.replace(".js", "");
    // attach the routes in it's own filename
    routing[filename] = require(path.join(__dirname, file));
  });

module.exports = routing;

drums.js and pianos.js content are exactly the same, just the "drums" and "pianos" change so we know we are reading from the correct file...

const express = require('express');

const router = express.Router();

const getAll = (req, res) => {
    res.send({ action: 'GET all drums' })
};
const create = (req, res) => {
    res.send({ action: 'POST drums', data: res.body })
};
const getById = (req, res) => {
    res.send({ action: 'GET drums by id', id: req.params.id })
};
const editById = (req, res) => {
    res.send({ action: 'PUT drums by id', id: req.params.id })
};
const deleteById = (req, res) => {
    res.send({ action: 'DEL drums by id', id: req.params.id })
};

router.get('/', getAll);
router.post('/', create);
router.get('/:id', getById);
router.put('/:id', editById);
router.delete('/:id', deleteById);

module.exports = router;

when you fire up the server up, you will get all this routes:

enter image description here

and output as

enter image description here

full project (though it's very simple) can be found on GitHub

balexandre
  • 73,608
  • 45
  • 233
  • 342