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

Getting the current chat room ID

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

Problem

I need roomId (an auto_increment primary key) to locate a specific room to update chat info. My way of doing it right now is dumb.

In createRoom.js, I use roomName from a form that a user has entered to create a room model and find its roomId, then make roomId a param of a URL:

var Room = require('../models/database').Room

exports.create = function (req, res) {

Room
    .create({
        room_name: req.body.roomName
    })
    .complete(function () {

        Room
            .find({where: {room_name: req.body.roomName}})
            .success(function (room) {

                res.redirect('rooms/videochat/' + req.body.roomName + '/' + room.room_id);

        })

    })

};


in routes.js, I grab roomId with req.params.roomId and pass it to a view file jade so it can be displayed on a video chat page:

app.get('/rooms/videochat/:roomName/:roomId', function (req, res) {
    res.render('videochat.jade', {roomName: req.params.roomName, roomId: req.params.roomId});
});


In the client side JavaScript, I use

var roomId = $('#roomId span').text();


to get the roomId, then use socket.emit to pass it back to server side:

socket.emit('updateStartTime', roomId);

Solution

Interesting question,

my first point is that it is not clear how you wrote the Room module, but ideally, that module should do 2 things:

  • Provide the created room to complete



  • Provide a way to deal with failure



Then, I would expect in the top code to see something like this:

var Room = require('../models/database').Room

exports.create = function (req, res) {
    Room.create({
        room_name: req.body.roomName
    })
    .complete(function ( success, room ) {
        if( success )
            res.redirect('rooms/videochat/' + room.roomName + '/' + room.roomId );
        else
            res.redirect('cuteapologetickittenvideo/');
    });
};


You will notice that I also changed req.body.roomName -> room.room_name -> room.roomName which makes more sense to me. On a general note I find it bad practice to require room_name in your call to the constructor, but roomName in the request. I would suggest to stick to 1 convention (The JavaScript One) and use roomName everywhere. Then you could write

var Room = require('../models/database').Room

exports.create = function (req, res) {
    Room.create( req.body ).complete(function roomComplete( success, room ) {
        if( success )
            res.redirect('rooms/videochat/' + room.roomName + '/' + room.roomId );
        else
            res.redirect('cuteapologetickittenvideo/');
    });
};


For the Jade processing, you should be able to get room id and name from the matching on '/rooms/videochat/:roomName/:roomId'.

Same for the client side javascript, I would be more inclined to get this from the URL than from a field in the HTML.

Code Snippets

var Room = require('../models/database').Room

exports.create = function (req, res) {
    Room.create({
        room_name: req.body.roomName
    })
    .complete(function ( success, room ) {
        if( success )
            res.redirect('rooms/videochat/' + room.roomName + '/' + room.roomId );
        else
            res.redirect('cuteapologetickittenvideo/');
    });
};
var Room = require('../models/database').Room

exports.create = function (req, res) {
    Room.create( req.body ).complete(function roomComplete( success, room ) {
        if( success )
            res.redirect('rooms/videochat/' + room.roomName + '/' + room.roomId );
        else
            res.redirect('cuteapologetickittenvideo/');
    });
};

Context

StackExchange Code Review Q#49157, answer score: 2

Revisions (0)

No revisions yet.