2

This while loop is causing my Angular app to stall. If I replace randomdrug in the last line with a simple string like 'foo', the list is populated. When I add randomdrug, the app stalls. What am I doing wrong?

  getDrugsForSale() {
    while (this.drugsforSale.length < this.locations[this.currentLocation].numDrugs) {
      const i = Math.floor(Math.random() * ((this.locations[this.currentLocation].numDrugs - 1) - 0) + 0);
      const randomdrug = this.drugNames[i];
      if (!(this.drugsforSale.includes(randomdrug))) {
        this.drugsforSale.push(randomdrug);
      }
    }
  }

5
  • What kind of size are we talking about with numDrugs ? Commented Feb 2, 2018 at 11:08
  • No greater than 13. Commented Feb 2, 2018 at 11:09
  • And the drugNames array how big is it ? Commented Feb 2, 2018 at 11:25
  • There are 13 drugs. Commented Feb 2, 2018 at 11:27
  • Ah ok that's the problem, I will come with an answer shortly. Commented Feb 2, 2018 at 11:28

1 Answer 1

2

The problem is that you have a number of drugs and you want to move them all in an array of similar size. random, might take a long time to actually get 13 distinct indexes.

For example if we have 13 drugs and we want to get 12 drugs in the result array, random will have to generate 12 distinct numbers, in the 1-13 range, it may take a long time to get all of them, random guarantees randomness not fair distribution. In large sets you will get all numbers, but the key is large

A better approach would be to copy the array and perform random swaps and then take the amount you need. This guarantees the time it takes is the same regardless of what random generates:

getDrugsForSale() {
    let max = this.drugNames.length;
    let result = this.drugNames.slice(0);
    for (let i = 0; i < 30; i++) {
        let source = Math.floor(Math.random() * (max - 1) - 0);
        let dest = Math.floor(Math.random() * (max - 1) - 0);
        let aux = result[source];
        result[source] = result[dest];
        result[dest] = aux;
    }

    return result.slice(0, this.locations[this.currentLocation].numDrugs);
}

Note A simple proof of the fact that it will not generate all the numbers easily, would be to put a console.log(i) in the original code and see the output.

Sign up to request clarification or add additional context in comments.

2 Comments

Beautiful! It will take me a min to understand, but it works.Thanks!
@BradHarris If you have any questions I can clarify :)

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.