current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 2 down vote favorite

I have an array that contains n arrays and each of these arrays contain a different number of string elements.
Each string contains a key word like evar#, event# or prop# (where # is a number).

This is my goal:

  • return all the key words from a string that is different with at least one of the other arrays
  • return the key words that does not exist in at least one of the other arrays.

I implemented a solution but it needs some optimization ... I'm open to any suggestions

Note: I'm using Google Apps Script which only accept ES5 (the "classic style").

This is my code:

var allActions = [
    [
        "overwrite value of evar1 with page_url_query 'int_cmp'", 
        "set event1 to custom value '1'", 
        "set event2 to custom value '1'", 
        "overwrite value of evar2 with page_url", 
        "overwrite value of evar3 with contextdata.user_id", 
        "set event4 to eventid", 
        "set event3 to eventid"

    ], 
    [
        "overwrite value of prop3 with contextdata.phase", 
        "overwrite value of prop2 with contextdata.room", 
        "set event1 to custom value '1'", 
        "set event2 to eventid", 
        "overwrite value of evar5 with contextdata.queue", 
        "set event4 to eventid", 
        "overwrite value of evar6 with contextdata.audience", 
        "set event3 to eventid",
        "set event5 to custom value '1'"
    ], 
    [
        "overwrite value of evar4 with contextdata.no_challenges", 
        "overwrite value of prop3 with contextdata.user_type", 
        "overwrite value of evar7 with contextdata.interaction", 
        "set event2 to custom value '1'", 
        "set event3 to eventid",
        "set event4 to eventid", 
        "set event1 to custom value '1'", 
        "set event5 to custom value '1'"
    ]
]

    
var keyWords = ["campaign","evar","event","prop", "mvvar1", "mvvar2", "mvvar3",
              "purchase", "scOpen", "scView", "scAdd"];

var arrLen = [];
var different = [];
for(var i = 0; i < allActions.length; i++) {
  arrLen.push(allActions[i].length);
} 

var max = Math.max.apply(null, arrLen)
var maxValue = arrLen.indexOf(max);

for(elem in allActions[maxValue]) {
  for(var i = 0; i < allActions.length; i++) {
    if(i !== maxValue) {
      for(var j in allActions[i]) {
        var mainElem = allActions[maxValue][elem];
        var checkElem = allActions[i][j];
        if(mainElem !== checkElem) {
          for(var k = 0; k < keyWords.length; ++k) {
            if( (index = mainElem.indexOf(keyWords[k])) !== -1) {
              splittedStr = mainElem.substring(index, mainElem.length).split(' ', 1);
              if(splittedStr[0].indexOf("campaign") !== -1) {
                if(different.indexOf(splittedStr[0]) == -1) {
                  different.push(splittedStr[0]);
                }
              }
              if(splittedStr[0].indexOf("evar") !== -1) {
                if(different.indexOf(splittedStr[0]) == -1) {
                  different.push(splittedStr[0]);
                }
              }
              if(splittedStr[0].indexOf("prop") !== -1) {
                if(different.indexOf(splittedStr[0]) == -1) {
                  different.push(splittedStr[0]);
                }
              }
              if(splittedStr[0].indexOf("event") !== -1) {
                if(different.indexOf(splittedStr[0]) == -1) {
                  different.push(splittedStr[0]);
                }
              }
            }

            else if( (index = checkElem.indexOf(keyWords[k])) !== -1) {
              splittedStr = checkElem.substring(index, checkElem.length).split(' ', 1);
              if(splittedStr[0].indexOf("campaign") !== -1) {
                if(different.indexOf(splittedStr[0]) == -1) {
                  different.push(splittedStr[0]);
                }
              }
              if(splittedStr[0].indexOf("evar") !== -1) {
                if(different.indexOf(splittedStr[0]) == -1) {
                  different.push(splittedStr[0]);
                }
              }
              if(splittedStr[0].indexOf("prop") !== -1) {
                if(different.indexOf(splittedStr[0]) == -1) {
                  different.push(splittedStr[0]);
                }
              }
              if(splittedStr[0].indexOf("event") !== -1) {
                if(different.indexOf(splittedStr[0]) == -1) {
                  different.push(splittedStr[0]);
                }
              }
            }
          }
        }
      }
    }
  }
}
console.log(different);

Can I somehow join these two conditions into a single condition? The first if statement checks the keyWord from the array with most values, and the second if statement checks the keyWord from the other arrays.

        if( (index = mainElem.indexOf(keyWords[k])) !== -1) {
          splittedStr = mainElem.substring(index, mainElem.length).split(' ', 1);
          if(splittedStr[0].indexOf("campaign") !== -1) {
            if(different.indexOf(splittedStr[0]) == -1) {
              different.push(splittedStr[0]);
            }
          }
          ..................
        }

        else if( (index = checkElem.indexOf(keyWords[k])) !== -1) {
          splittedStr = checkElem.substring(index, checkElem.length).split(' ', 1);
          if(splittedStr[0].indexOf("campaign") !== -1) {
            if(different.indexOf(splittedStr[0]) == -1) {
              different.push(splittedStr[0]);
            }
          }
          ...................
        }
share|improve this question
up vote 0 down vote

To improve your code you need apply more the Single responsibility.

You don't need to reduce theses two conditionals to one, you can create a function and call this function in both.

var handleString = function(splittedStr, resultArray) {
    if(splittedStr[0].indexOf("campaign") !== -1) {
      // push the campaing
    }
    if(splittedStr[0].indexOf("evar") !== -1) {
      // push the evar
    }
    // continue
}

In this way your function will have less repeated code but your intention writing it remains the same, so in the future you (and your co-workers) will understand it better.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.