Equalized PRNG number distribution

Question

The main idea behind this code is to distribute multiple random numbers, but guarantee that the probability of each coming is equal.

The code is really basic:

( function( window ){
 window.RNG = function(min, max){
 if( min / 1 != min )
 {
 throw new TypeError('Expecting Number or NumericString, got ' + ( min != min ? 'NaN' : typeof min ) );
 }
 if(arguments.length > 1)
 {
 if( max / 1 != max )
 {
 throw new TypeError('Expecting Number or NumericString as the second parameter, got ' + typeof max);
 }
 else if( ( min = min >> 0 ) >= ( max = max >> 0 ) )
 {
 throw new RangeError('The 2nd parameter should be higher than the first');
 }
 }
 else
 {
 max = min << 0;
 min = 0;
 }
 var dif = max - min, refill=function(){
 for(var i = min, result={length:0}; i <= max; i++)
 {
 result[i]=true;
 result.length++;
 }
 return result;
 }, avail = refill();
 return {
 valueOf:function(){
 if( !avail.length )
 {
 avail = refill();
 }
 if( avail.length > 1 )
 {
 var result;
 do
 {
 result = ( window.Math.random() * ( dif + 1 ) + min ) << 0;
 }
 while( !avail[result] );
 delete avail[result];
 avail.length--;
 return result;
 }
 else
 {
 for(var k in avail)
 {
 if(k != 'length')
 {
 break;
 }
 }
 avail = refill();
 return k / 1;
 }
 },
 toString:function(){
 return this.valueOf() + '';
 }
 };
 };
} )( Function('return this')() );

The reason why I use that Function('return this')() is just to ensure I have the actual window object.

In terms of readability, what can I improve?

Is there what bad practices that I'm using?

• Thriggle: Regarding readability, you've written "Excepting" when you meant "Expecting" in your type error messages.
• Ismael Miguel: @Thriggle Thank you. I didn't notice that typo. I have fixed it in the question. Don't delete your comment, since it is important.

Answer 1

I can't comment on JS in particular too much because I'm no expert. I'll leave that for others more qualified. I can offer some advice on general programming principles though.

Code Styling

Code styling is a preference and yours is fairly consistent at least but since you asked about readability, I will talk a bit about it. Because code styling is generally considered to be opinionated, I will try not to be too specific and stick to the generally accepted norms.

Place opening braces on the same line as the block statement they belong to. There is no need to take up a whole line for a single character. Use white space to format for neatness where appropriate instead. Same for else blocks. They belong to the if and cannot exist on their own.

if ( foo == bar ) {
} else if ( foo == foobar ) {
} else {
}

Also, space out your syntax appropriately. You should have a space before the opening brace.

window.RNG = function( min, max ) {

'if', 'for', 'while', etc are not functions. place a space between the keyword and the conditional (). See the 'if' example above. Do not place a space between function identifiers and the argument list (you are already doing this).

If you mix variable declarations and assignments, do not use a list of declarations. Create a separate declaration for each.

var dif = max - min, refill=function(){
 for (var i = min, result={length:0}; i <= max; i++) {
 result[i]=true;
 result.length++;
 }
 return result;
}, avail = refill();

... is better written as ...

var dif = max - min;
var refill = function() {
 for ( var i = min, result = { length:0 }; i <= max; i++ ) {
 result[i] = true;
 result.length++;
 }
 return result;
};
var avail = refill();

Programming Habits

Please don't do this. Mixing assignments in expressions is almost always a bad idea.

else if( ( min = min >> 0 ) >= ( max = max >> 0 ) )

You are essentially doing 5 different things in a single statement. This is very confusing and exceptionally prone to error. Without detailed inspection, it is not apparent that you are calculating and assigning min and max at the same time you are comparing them. Not good. Figure out how to split it into 3 statements: 2 assignments and the if statement with a simple conditional.

Why are you calculating dif inside of this expression and inside of a loop? The value of dif never changes.

result = ( window.Math.random() * ( dif + 1 ) + min ) << 0;

Precalculate whenever possible:

var dif = ( max - min ) + 1;

What is this? What's wrong with isNaN()? Try to avoid trickery. It's difficult to know what you intend to happen here. I only know what it's supposed to do by reading the error message.

if( min / 1 != min )

Also use parentheses to make your precedence explicit. Try not to rely on operator precedence. It requires deeper knowledge and is prone to mistakes. Also, the reader can't know if you made a logical error. In the above case we can assume what the precedence is supposed to be because it generally doesn't make sense to return a value to a conditional. That is not always the case though and you make the reader think harder than they should need to.

It has been raised that isNan() is not always available. From my own link, the comments also indicate that isNaN() will not work in every case. If these issues are a concern, then I would suggest creating a new function to self-document what is being done. Since the same test is used in two different places, it is a good candidate for it's own function anyway.

• Ismael Miguel: If you notice well, I use the vqriable dif 3 times and that only time is when I use dif + 1. That was the only time you refered to calculate it.I agree and disagree with your note about isNaN, but that function isn't available everywhere and it's pretty reliable to assume that if something isn't equal to itself, it is NaN.
• Fuzzy Logic: Sorry, I don't see where else you are using dif. I've search your code for 'dif'. There is only two occurrences; the declaration and the (dif + 1) expression. Re: isNaN(), why don't you make your own isNaN() function then, that does exactly what you have? I don't see an issue with your method, just the readability.
• Ismael Miguel: I agree with the readability issue. I've analised a bit better the code and you tell me to space the arguments, but on your example code there is no spacing. Also, I've seen somewhere refered to keep the declarations together.
• Fuzzy Logic: I'm guessing you mean, the 'if' example where I wrote if (foo == bar) { instead of if ( foo == bar ) {? That is just personal preference and that's where code styling becomes opinionated :) There's always a "yes, but..". I suggest you follow your own advice in this case, not mine. I will update my answer to remove my preference, since it's subjective.
• Fuzzy Logic: Concerning keeping declarations together, I expect they meant that you shouldn't sprinkle them in your code. Keep them at the top, before execution happens. This is subjective too though. Some people will argue the opposite because it makes sense to keep declarations as close as possible to the place where they are used, then release the variable/resources immediately after. You need to use good judgement. I would argue that you should aim to keep them at the top and if you find that you need to place them in the middle of your code, you should probably refactor your code.