mint-metrics / mojito-js-delivery

🧪 Source-controlled JS split testing framework for building and launching A/B tests.
https://mojito.mx/docs/js-delivery-intro
Other
16 stars 29 forks source link

Refactor chosenRecipe setting inside activate() function for inTest users #39

Closed kingo55 closed 4 years ago

kingo55 commented 4 years ago

In @dapperdrop and my review yesterday, we found the logic on recipe selection to be overly verbose:

                    this.setInTest(1);

                    // TODO: optimise logic?
                    var chosenRecipe,
                        currentRecipe = this.getRecipe();

                    if (!currentRecipe)
                    {
                        chosenRecipe = this.chooseRecipe();
                    }
                    else
                    {
                        // Returning visitors see the same recipe as last time
                        chosenRecipe = currentRecipe;
                    }

                    this.setRecipe(chosenRecipe);

                    var chosenRecipeObject = this.options.recipes[chosenRecipe];

                    // Exclude user from test, if chosen recipe no longer exists
                    if (!chosenRecipeObject)
                    {
                        this.setInTest(0);
                        return success;
                    }

I think something like this could work, but @allmywant would probably know a better way to handle this.

                    var chosenRecipe = this.getRecipe() || this.chooseRecipe();
                    this.setRecipe(chosenRecipe);

                    // Exclude users from test if their chosen recipe no longer exists
                    var chosenRecipeObject = this.options.recipes[chosenRecipe];
                    if (!chosenRecipeObject)
                    {
                        this.setInTest(0);
                        return success;
                    }
                    else this.setInTest(1);
allmywant commented 4 years ago

@kingo55 I think your code looks good enough, I can't think of a cleaner way to do this :)

dapperdrop commented 4 years ago

I will update this in https://github.com/mint-metrics/mojito-js-delivery/pull/37