feat: Add JS experiment client #26
Conversation
ayushjain17
force-pushed
the
jsExpClient
branch
2 times, most recently
from
51009ee to
9607799
Compare
clients/js/src/index.ts
Outdated
| if (toss >= range) { | ||
| return undefined; | ||
| } | ||
| let buckets = Array.apply(null, Array(variant_count)).map(function (_, i) { return traffic * (i + 1); }); |
There was a problem hiding this comment.
This is fine, but too computations happening here, making an array of NULLs, mapping over it, and then in the finding the index of the bucket and finally using the index to get the variant/bucket.
I think this can be just written in one single for loop.
clients/js/src/index.ts
Outdated
| @@ -39,3 +39,61 @@ export class CacReader { | |||
| return deepMerge(targetConfig, ...requiredOverrides); | |||
| } | |||
| } | |||
|
|
|||
| export class ExperimentReader { | |||
| experiments: ExperimentStore; | |||
There was a problem hiding this comment.
ExperimentStore is not really required, you could just use Experiments here
clients/js/src/index.ts
Outdated
| const experiments = []; | ||
| const iter = this.experiments.values(); | ||
|
|
||
| while (true) { |
There was a problem hiding this comment.
Can we just use a for loop? Infinite loops are dangerous and there are a limited number of experiments
| this.experiments = new Map(experiments.map(exp => [exp.id, exp])); | ||
| } | ||
|
|
||
| public getApplicableVariant(data: IObject, toss: number): Array<String> { |
There was a problem hiding this comment.
the type number allows floating point values too, can we add an assertion that toss is always a positive integer?
There was a problem hiding this comment.
Can we add the toss isInteger check here?
ayushjain17
force-pushed
the
jsExpClient
branch
2 times, most recently
from
d793442 to
859be27
Compare
| this.experiments = new Map(experiments.map(exp => [exp.id, exp])); | ||
| } | ||
|
|
||
| public getApplicableVariant(data: IObject, toss: number): Array<String> { |
There was a problem hiding this comment.
Can we add the toss isInteger check here?
Problem
Experimentation client not present in JS
Solution
Add Experimentation client in JS