amark / mongous

Simple MongoDB driver for Node.js with jQuery like syntax.
MIT License
246 stars 30 forks source link

mongous.prototype.id #21

Closed acsaga closed 12 years ago

acsaga commented 12 years ago

I'm curious why use

Math.round(Math.exp(Math.random() * Math.log(100000)));

to generate id. Is there any mathematical background behind the formula? or the 'net' module require an id like that?

after a few attempts, I thought this formula is not very good as an random id generator. It has much bigger chance to get a number between 1-10 than other numbers.

try this code in node:

for(var i=0;i<20;i++) {
    var t = Math.round(Math.exp(Math.random() * Math.log(100000)));
    console.log( t );
}

every time I executed the code, I could find numbers between 1-10. That means if I send many requests to mongous in a short time, the id has a great chance to conflict and one request may trigger the callback which belongs to another request.

amark commented 12 years ago

Edit: Deleted my last post.

You are correct. Sorry. That was just a quick really-random function I had either found or written, but I wasn't aware that it doesn't work very well as you have noted. I don't use it in anymore, though. I have a different really-random function, but it outputs a string, and warning! does not work well with parseInt();

// returns a string of 'l' length, must <= 14 in length. works badly with parseInt()
function (l){return (Math.pow(Math.random(),Math.random()).toString().slice(-(l||6)))}

Pulling your update now.

acsaga commented 12 years ago

thanks for your reply :)

In my case, I found simply using Math.round( Math.random() * 80000 ) is ok. it generates an integer between 0-80000.

amark commented 12 years ago

Yeah, that looks like a safer fix.

Thank you for noticing it and updating. Greatly appreciated! Sorry for being so lazy.