Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This is a very simple function, but it gets run on a roughly 10ms interval, so I'm looking for ways to improve it. The first obvious thing is that it has to run the comparisons nearly every time as it starts with the lowest probabilities first. Is there a way I can reverse that without breaking it, or a way I can generally improve the performance of this function to achieve the same end result?

function find_coin() {
    var roll = Math.random() * 100;

    if (roll < 0.01)
        currency['platinum']++;
    else if (roll < 0.1)
        currency['gold']++;
    else if (roll < 1)
        currency['silver']++;
    else if (roll < 10)
        currency['bronze']++;
};
share|improve this question

2 Answers 2

up vote 7 down vote accepted

Reversing the order of the tests is probably your best option here. In your current code:

function find_coin() {
    var roll = Math.random() * 100;

    if (roll < 0.01)
        currency['platinum']++;
    else if (roll < 0.1)
        currency['gold']++;
    else if (roll < 1)
        currency['silver']++;
    else if (roll < 10)
        currency['bronze']++;
};

90% of your random values will be >= 10, additionally, 9 % will be between 1 and 10 %, so, 99% of your tests will have to check.... 4 roll conditions (< 0.01, < 0.1, < 1, and < 10). So, 99% of all your rolls will check 100% of your conditions.

If you reverse the logic, and do:

function find_coin() {
    var roll = Math.random() * 100;

    if (roll >= 10) {
        return
    }

    if (roll >= 1) { 
        currency['bronze']++;
    } else if (roll >= 0.1) {
        currency['silver']++;
    } else if (roll >= 0.01) {
        currency['gold']++;
    } else {
        currency['platinum']++;
    }

};

With the above code, 90% of your rolls will do just one test, 9% will do 2 tests, 0.9 will do 3 tests, and so on.

Note, also, that I have used braces even for the 1-liner blocks in the conditions. This is a good practice and it avoids bugs when maintaining your code, later.

share|improve this answer
    
Thank you! I've been looking at the last condition for 'platinum' and I'm wondering if the condition for roll < 10 is necessary? We know that roll < 10, and it is not >= 0.01, ergo I could forgo the last comparison? –  njp Jul 29 at 9:15
    
@njp - you are absolutely right, the last condition, does not need a condition at all, and it is just a copy/paste/brainfart that it is there at all, It should just be an else. –  rolfl Jul 29 at 12:30

Depending on your requirements and your performance needs you can pre-populate a lookup table of random numbers and implement flip_coin() as a table lookup:

function flip_coin() {
  return ++i >= lookupTable.length ? lookupTable[i=0] : lookupTable[i];
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.