2

I am working on a project where there I have to create team for user and but before that I need to check if that team name was already occupied by that same user.Below is sample code

 var mysql=require('../../config/mysql_connect.js');
 var dateFormat = require('dateformat');

 var functions={

    createTeam:function(data,cb){

        this.checkifExists(data,function(err,result){

            if(err)
            {
                cb(err);
            }
            else
            {
                if(result.length)
                {
                    cb(null,0); // Team name exists 
                }
                else
                {
                    mysql.getConnectionFromPool(function(err,con){

                        if(err)
                        {
                            cb(err);
                        }
                        else
                        {

                            var query=con.query("Insert into team SET ? ",data,function(err,result){

                            if(err)
                            {
                                cb(err);
                            }
                            else
                            {
                                cb(null,result.insertId);
                            }

                                con.release();
                            });

                           }
                        });
                }
            }
        });


    },
    checkifExists:function(data,cb){

        mysql.getConnectionFromPool(function(err,con){

            if(err)
            {
                cb(err);
            }
            else
            {
                var sql="Select id from team where user_id = ? and team_name like  "+ con.escape('%'+data.team_name+'%') ;  

                var query=con.query(sql,[data.user_id,data.team_name],function(err,result){

                    if(err)
                    {
                        cb(err);
                    }
                    else
                    {
                      cb(null,result);
                    }
                     con.release();
                });

                console.log(query.sql);
            }

        });
    }
};

module.exports=functions;

Everything works fine but is there any easier way to write this dependable queries in more manageable manner because there are times when there are more than 3-4 queries and code gets complicated and un-manageable.

I know same thing can be achieved via mysql unique index but still what if there are more then 3-4 queries and in some case indexes won't satisfy my needs

3
  • Your question is puzzling. Please edit it to clarify. Do you mean dependent queries where you wrote dependable? It's hard to tell from your code what you are trying to do. Have you considered using unique indexes to enforce such things as unique team names? Commented Sep 11, 2016 at 12:00
  • I don't want to add indexes on table .Dependable means result of one query will be used by other query and so on. eg : In here I checked if unique team name exists and if true other query wont fire but in some cases I may need to fire some other queries Commented Sep 11, 2016 at 12:46
  • The jargon for this is constraints. Most SQL developers use unique indexes for the purpose. Apparently your requirements are different from those of most SQL developers. You're going to need to learn about using transactions. Commented Sep 11, 2016 at 14:11

2 Answers 2

1

As I can see there are two problems.

  1. You can't use transaction with unmanaged pool connection because transaction started for one connection can't be passed to other.
  2. You get and release connection for each function. It's unnecessary duplicating code.

To solve this these problems you can use separate connection via entity e.g. one connection for team, second for user and more. To avoid callback hell use async module or promises.

This is draft

// db.js
var mysql=require('../../config/mysql_connect.js');
var connections = {};

// No, that code don't solve problems, because transaction can't started inside other.
// Block mechanism by transaction property `is_busy` seems a bulky.
function DB(name) {
    if (!name)
        // unnamed connection. Must be released on query end.
        return mysql.getConnectionFromPool(); 

    if (!connections[name])
        // create always open connection
        connections[name] = mysql.getConnectionFromPool();

    return connections[name];
}

module.exports = DB;

// team.js
var dateFormat = require('dateformat');
var async = require('async');
var db = require('db')('team'); // get always-open connection for team-entity from pool

function create (data, cb) {
    // Here async is not necessary but used as example
    async.waterfall([
        function(cb) {
            isExists(data.user_id, data.team_name, cb);
        },  

        function(isTeamExists, cb) {
            if (!isTeamExists)
                return cb(); // go to end of waterfall
            // is a bad statement; use enum of fields
            db.query('insert into team SET ?', data, cb); 
        }],

        // end waterfall chain
        function(err, team_id) {
            if (err)
                return cb(err); // pass error to original cb-func

            ...do-smth and call cb without error... 
        }
    );  
}

function isExists (user_id, team_name, cb) {
    // You can use ?? to masked input
    db.query('select 1 from team where user_id = ?? and team_name like "%??%"', 
        [user_id, team_name], 
        function(err, res) {
            cb(err, !err && res.length > 0);
        }
    );
}

module.exports = {
    create,
    isExists
};
Sign up to request clarification or add additional context in comments.

Comments

1

This is the basic problem everyone has with JavaScript. I have tried many different ways of solving this over the years.

After experimenting with many options, I believe the best way (that is currently largely accepted) is to use babel with the async/await keywords. If you use the es2017 preset that is one way to ensure that is available.

Now the disclaimer to that is that you will still have to learn promises. If you try to skip over promises to async/await you will quickly find a bunch of promise-based code that does not make sense. Also, async/await requires promises..so you will need to teach yourself that. It will take a bit of time to get used to it.

Another thing that can help in general with JavaScript is to use two spaces instead of four or a tab. That does make your code significantly more readable. Also, place your open curly braces on the same line, and don't add extra blank lines everywhere, only where they are necessary for making the code easier to read.

So, this is not exact code, but an outline of how I would do it:

async function existsTeam(data) {
  const id = await query('select id ..', data);
  return id;
}

async function createTeam(data) {
  const existingId = await existsTeam(data);
  if (existingId) return existingId;

  const result = await query('insert into team ..', data);
  return result.insertId;
}

For the specific case of MySQL, you are trying to enforce a constraint, which I think that MySQL doesn't support foreign key constraints on the db -- but you might double check because if it does that could simplify things. But these types of queries/updates may be easier with Sequelize or Bookshelf ORMs.

3 Comments

thanks man just one thing what is right and optimised way for this task use async or promises like q.If async indirectly uses promises so is it better to learn promises after all.
Well just to make sure, we have to be careful about the keyword async because as you used it there is not clear. It could refer to async code in general (i.e. how Node always works, asynchronous vs synchronous), or the async module, or the async and await keywords. The async and await code is obviously (to me) cleaner. Performance is usually similar. Do not recommend q use native promises or bluebird. I recommend learn promises and then do it as I showed.
About 'optimised' in this context. The thing you must realize is that the amount of time for network to hit the database will likely be 100-1000 plus times more than any computation. gist.github.com/jboner/2841832 -- must read this carefully. Having said that, if you execute these types of functions in a loop without network overhead, exact implementation (final transpiled output) and Node/v8 engine version can make a huge difference in the types of optimizations the JIT compiler makes. That is not relevant in this type of case -- you wait for network, CPU is negligible.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.