Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have an Apps Script that is supposed to run through the spreadsheet, and if a company belongs to a certain country (there's a country column), set the value of the region row to something (e.g. Africa).

The spreadsheet has more than 8500 rows and this is the dirty code I have. It's working, but if there's a better way to write it, I'd appreciate any pointers:

function setRegions() {
      var mySheet = SpreadsheetApp.getActiveSpreadsheet().getActiveSheet();
      var myCompany = mySheet.getDataRange().getValues();
      var start = 2;
      for(i = 1; i < myCompany.length; i++){
        var univ = myCompany[i];
        var country_code = univ[4];
        if (country_code == "ke" || 
            country_code == "ng" || 
            country_code == "ug" || 
            country_code == "za" ||
            country_code == "sn" ||
            country_code == "gh") {
          rgMyRange = mySheet.getRange("M" + start);
          rgMyRange.setValue("AFRICA");
        }else if (country_code == "eg" ||
                 country_code == "sa" ||
                 country_code == "ma"){
          rgMyRange = mySheet.getRange("M" + start);
          rgMyRange.setValue("MENA");
        }else if (country_code == "br" ||
                 country_code == "ar" ||
                 country_code == "mx" ||
                 country_code == "pe" ||
                 country_code == "co"){
          rgMyRange = mySheet.getRange("M" + start);
          rgMyRange.setValue("LATAM");
        }else if (country_code == "my" ||
                 country_code == "th" ||
                 country_code == "id" ||
                 country_code == "ph"){
          rgMyRange = mySheet.getRange("M" + start);
          rgMyRange.setValue("APAC");
        }else if (country_code == "in") {
          rgMyRange = mySheet.getRange("M" + start);
          rgMyRange.setValue("IN");
        }
        start++;
      }
    }
share|improve this question

1 Answer 1

up vote 0 down vote accepted

TL;DR : My comments won't improve the performances much. Also, I've never used Apps Script.

This being said, here what I did : I decided to change details to make your code more concise (removing variables used only once for instance).

function setRegions() {
      var mySheet = SpreadsheetApp.getActiveSpreadsheet().getActiveSheet();
      var myCompany = mySheet.getDataRange().getValues();
      for(i = 1, start = 2; i < myCompany.length; i++, start++){
        var country_code = myCompany[i][4];
        if (country_code == "ke" || 
            country_code == "ng" || 
            country_code == "ug" || 
            country_code == "za" ||
            country_code == "sn" ||
            country_code == "gh") {
          mySheet.getRange("M" + start).setValue("AFRICA");
        }else if (country_code == "eg" ||
                 country_code == "sa" ||
                 country_code == "ma"){
          mySheet.getRange("M" + start).setValue("MENA");
        }else if (country_code == "br" ||
                 country_code == "ar" ||
                 country_code == "mx" ||
                 country_code == "pe" ||
                 country_code == "co"){
          mySheet.getRange("M" + start).setValue("LATAM");
        }else if (country_code == "my" ||
                 country_code == "th" ||
                 country_code == "id" ||
                 country_code == "ph"){
          mySheet.getRange("M" + start).setValue("APAC");
        }else if (country_code == "in") {
          mySheet.getRange("M" + start).setValue("IN");
        }
      }
    }

Then, you can get rid of the different tests on the country_code by storing what you want in a dictionary :

function setRegions() {
      var dict = {
          "ke" : "AFRICA",
          "ng" : "AFRICA",
          "ug" : "AFRICA",
          "za" : "AFRICA",
          "sn" : "AFRICA",
          "gh" : "AFRICA",
          "eg" : "MENA",
          "sa" : "MENA",
          "ma" : "MENA",
          "br" : "LATAM",
          "ar" : "LATAM",
          "mx" : "LATAM",
          "pe" : "LATAM",
          "co" : "LATAM",
          "my" : "APAC",
          "th" : "APAC",
          "id" : "APAC",
          "ph" : "APAC",
          "in" : "IN",
      };
      var mySheet = SpreadsheetApp.getActiveSpreadsheet().getActiveSheet();
      var myCompany = mySheet.getDataRange().getValues();
      for(i = 1, start = 2; i < myCompany.length; i++, start++){
        var country_code = myCompany[i][4];
        if (dict[country_code])
        {
          mySheet.getRange("M" + start).setValue(dict[country_code]);
        }
      }
    }
share|improve this answer
    
Apps Script implements Dicts like this: var dict = { "ke" : "AFRICA", "ng" : "AFRICA", "ug" : "AFRICA", "za" : "AFRICA", "sn" : "AFRICA", "gh" : "AFRICA", "eg" : "MENA", "sa" : "MENA", "ma" : "MENA", "br" : "LATAM", "ar" : "LATAM", "mx" : "LATAM", "pe" : "LATAM", "co" : "LATAM", "my" : "APAC", "th" : "APAC", "id" : "APAC", "ph" : "APAC", "in" : "IN", }; –  jwesonga Feb 13 '13 at 12:19
    
+Josay your code works like a charm, I'm going to run it on the 8000 records spreadsheet and see how it performs –  jwesonga Feb 13 '13 at 12:27
    
Glad you like it. I've updated my code to take your comment into account. Not sure if perf will be better. –  Josay Feb 13 '13 at 22:29
    
Keeping the code at array level should speed things up, rather than run the setValue within the loop. I'm modifying it to see if it's going to perform better. –  jwesonga Feb 15 '13 at 6:46
    
Not quite sure what the bottleneck is here. –  Josay Feb 15 '13 at 7:44

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.