The code I have now is currently working, but I'm looking for ways to optimize it, and really, to just get any feedback.I'm implementing this code through Ensighten (a tag management platform) as a custom javascript tag. Ensighten works by injecting tags either sync or async scripts on to a page. I've set this tag to fire after the DOM is ready. What it does is creates a namespace, and then sets a function in that namespace to main (see main.js). The main function takes one argument, an object (see lookupTable.js), in which it takes that object, loops through the object, and if the object contains the correct key, it sets that key's value equal to campaignCode. Next, the function looks to see if the campaignCode is not falsey, it then looks at the path (window.location.path) to see if it's either a courses or a topics page. If it's a topics page it fires the analytics code. If it's a courses page, it listens for a message from an iframe that a video was clicked. If the message from that iframe equals play button clicked then it fires the other analytics code.The main function is called from the the lookupTable.js. Like so:window.analytics.campaign.main(window.analytics.campaign.lookupTable);Questions:Should I be checking if the main function's argument is an object?Is there a way I wouldn't have to check if the campaignCode is falsy? Could I check it in the if statement of the for loop?Am I declaring the analyticsTrack function in the correct way?main.js/*jslint browser:true*//*globals Bootstrapper*/window.analytics = window.analytics || {};window.analytics.campaign = window.analytics.campaign || {};window.analytics.campaign.main = function (lookupTable) {    var path = window.location.pathname,        campaignCode = '',        prop;    function analyticsTrack (id) {        return Bootstrapper.insertScript('//www.example.com/click/services/Tracking/Click.ashx?CR_EAC=' + id);    }    for (prop in lookupTable) {        if (lookupTable.hasOwnProperty(prop) && path === prop) {            campaignCode = lookupTable[prop];        }    }    // check campaignCode    if (campaignCode) {        // if it's a topic page, fire a RIO tag onload        if (path.indexOf('topics') > -1) {            analyticsTrack(campaignCode);        }        // if it's a courses page        if (path.indexOf('courses') > -1) {            // listen for a message from the iframe            $(window).on('message', function (e) {                // check the message from the iframe, fire a RIO tag                if (e.originalEvent.data === 'play button clicked') {                    analyticsTrack(campaignCode);                }            });        }    }};lookupTable.jswindow.analytics = window.analytics || {};window.analytics.campaign = window.analytics.campaign || {};var campaign = window.analytics.campaign;campaign.lookupTable = {    '/topics/123456':    '300257956',    '/courses/234567':   '300257957',    '/courses/34567':    '300257958',    '/courses/15677789': '300257959',    '/topics/123457':    '300257970'};campaign.main(campaign.lookupTable);
Loop through object and send analytics code based on objects properties value
javascript
Should I be checking if the main function's argument is an object?No, document it as it accepts objects, the params are straight forward no need to check them for the consumerIs there a way I wouldn't have to check if the campaignCode is falsy? Could I check it in the if statement of the for loop?No need to loop just access the property directlycampaignCode = lookupTable[prop];Am I declaring the analyticsTrack function in the correct way?Yes and no, every time you invoke the main function you are allocating the method.  There are no closures around it so it doesn't need to be done.  Just move it outside of the main method but wrap everything in a self invoking method (if you are not already with some other tool) so it won't be global.  If I had to rewrite everything here is how I would do it.  Instead of having a bunch of if path contains x then do something with the campainCode statements I  just put it in an object.  It might be overkill but it is a good pattern to use in other places.  Say you want to handle books instead of adding another if indexof .. just add books to the pathsHandlers object.  (function() {    function analyticsTrack(id) {        return Bootstrapper.insertScript('//www.example.com/click/services/Tracking/Click.ashx?CR_EAC=' + id);    }    var pathHandlers = {        topics: function(campaignCode) {            analyticsTrack(campaignCode)        },        courses: function(campaignCode) {            $(window).on('message', function(e) {                // check the message from the iframe, fire a RIO tag                if (e.originalEvent.data === 'play button clicked') {                    analyticsTrack(campaignCode);                }            });        }    }    window.analytics.campaign.main = function(lookupTable) {        var path = window.location.pathname,            campaignCode = lookupTable[path],            handlerKey;        if (campaignCode) {            for (handlerKey in pathHandlers) {                if (pathHandlers.hasOwnProperty(handlerKey) && path.indexOf(handlerKey) > -1) {                    pathHandlers[handlerKey](campaignCode);                }            }        }    };})();--edit to answer commentYour function was defined in the main function but it didn't need to be because it wasn't using any locally scoped variables in main (no variables being accessed in the function via closure).If the method was for example :window.analytics.campaign.main = function (lookupTable) {    var path = window.location.pathname,        campaignCode = '',        prop;    function analyticsTrack (id) {        console.log('current path is ' + path)        return Bootstrapper.insertScript('//www.example.com/click/services/Tracking/Click.ashx?CR_EAC=' + id);    }The method couldn't be moved outside of main because it is referencing the path variable within a closure.  