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 30 forks source link

Refactor test objects' defaults setting #61

Closed kingo55 closed 4 years ago

kingo55 commented 4 years ago

The logic that manages testObject/Library specific overrides seems to be redundant for the storage and decision adapters, I think:

            // Storage adapter definition priority: 1) Test adapter; 2) Global adapter; 3) Default adapter
            if (!this.options.storageAdapter)
            {
                if (this.options.options && this.options.options.storageAdapter)
                {
                    this.options.storageAdapter = this.options.options.storageAdapter;
                }
                else
                {
                    this.options.storageAdapter = {
                        onExposure: function () {},
                        onRecipeFailure: function () {},
                        onTimeoutFailure: function () {}
                    };
                }
            }

            // Decision adapter definition priority: 1) Test adapter; 2) Global adapter; 3) Default adapter
            if (!this.options.decisionAdapter)
            {
                if (this.options.options && this.options.options.decisionAdapter)
                {
                    this.options.decisionAdapter = this.options.options.decisionAdapter;
                }
                else
                {
                    this.options.decisionAdapter = function(testObject)
                    {
                        return testObject.getSeededRNGRandom();
                    };
                }
            }

https://github.com/kingo55/mojito-js-delivery/blob/9f313bd1ba59f1f8dcb80cc4df41b6b02b08f28c/lib/mojito.js#L97-L128

This is because at the start of the object's constructor we handle the overrides here:

            // Test object specified options
            if (options.options)
            {
                for (p in Options)
                {
                    if (!(p in options.options))
                    {
                        options.options[p] = Options[p];
                    }
                }
            }
            else
            {
                this.options.options = {};

                for (p in Options)
                {
                    this.options.options[p] = Options[p];
                }
            }

All we need to do is make sure whatever option is specified gets reassigned:

this.options.storageAdapter = this.options.options.storageAdapter;

Unless I'm missing something?

kingo55 commented 4 years ago

addressed by #77