3
\$\begingroup\$

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);
\$\endgroup\$
  • 1
    \$\begingroup\$ It seems like your code does not work as it will not fire socket.on('disconnect'), which makes it off-topic for CR. \$\endgroup\$ – konijn May 7 '14 at 15:32
  • \$\begingroup\$ Hi, it will fire when users click close window button. \$\endgroup\$ – shangsunset May 7 '14 at 15:55
  • \$\begingroup\$ @konijn the code works as it can get roomId successfully. i just want to see if there is a bette way. it does not contain broken code \$\endgroup\$ – shangsunset May 9 '14 at 15:49
  • \$\begingroup\$ Voting to re-open now \$\endgroup\$ – konijn May 9 '14 at 15:54
  • \$\begingroup\$ you have a lot of Empty White Space in that code, you should really get rid of that in your code. \$\endgroup\$ – Malachi May 9 '14 at 17:41
2
\$\begingroup\$

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:

  1. Provide the created room to complete
  2. 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.

| improve this answer | |
\$\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.