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?

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.