dimanche 8 mai 2022

Typescript random number generation problem

This is driving me crazy as I can't see why this doesn't work! The requirement is simple: Robot should have a new unique name after its name is reset (by calling resetName()). My logic is to continuously generate a new name and the uniqueness is enforced by a Set. But it doesn't work, namely, if I create a Robot instance, after calling resetName 10000 times, then there should have been 10001 unique names. But it doesn't; only around 9920 names, give or take as it's random, have been generated indicated by the _counter variable. Can't figure out why.

export class Robot {
  private capitals = [...Array(26).keys()].map(i => String.fromCharCode(i + 65)); // generate 26 capital letters.
  private _name = '';
  private static _seen = new Set<string>();
  private _counter: number = 0;
  constructor() {}

    // name is in the format of XXYYY where X is a captial letter and Y is a digit.
  public get name(): string {
    if (!this._name) {
      // continuously generate a new unique name.
      while (!Robot._seen.has(this._name)) {
        this._name = this.capitals[this.randomNumber(25)] + this.capitals[this.randomNumber(25)] + this.randomNumber(9) + this.randomNumber(9) + this.randomNumber(9);
        if (!Robot._seen.has(this._name)) {
          this._counter += 1;
          Robot._seen.add(this._name);
          break;
        } 
      }
    }
    
    return this._name;
  }

  public resetName(): void {
    this._name = '';
  }

  public static releaseNames(): void {
    //Robot._seen.clear();
  }
  
  // generate a random number between min and max inclusively.
  private randomNumber = (max: number, min: number = 0): number => Math.floor(Math.random()*(max - min + 1) + min);
}

This is the unit test that failed:

it('should set a unique name after reset', () => {
    const NUMBER_OF_ROBOTS = 10000
    const usedNames = new Set()

    usedNames.add(robot.name)
    for (let i = 0; i < NUMBER_OF_ROBOTS; i++) {
      robot.resetName()
      usedNames.add(robot.name)
    }

    expect(usedNames.size).toEqual(NUMBER_OF_ROBOTS + 1)
  })

---------------- UPDATE ----------------- The accepted answer is right that I messed up my logic in the while loop. In fact the whole logic can be simplified as below:

public get name(): string {
    while (!this._name || Robot._seen.has(this._name)) {
      this._name = this.capitals[this.randomNumber(25)] + this.capitals[this.randomNumber(25)] + this.randomNumber(9) + this.randomNumber(9) + this.randomNumber(9);
    }
    Robot._seen.add(this._name);
    return this._name;
  }



Aucun commentaire:

Enregistrer un commentaire