3
\$\begingroup\$

When doing a search in Google, it is possible to have an ad in first position, at the top of the page or only on the first page. To do this, it is possible to pay for specific keywords for certain campaigns. For example, if I'm selling groom dresses, I'm going to want to invest in words like 'dress', 'married' and 'white'. When a campaign's CPC is manual, it's possible to adjust its CPC based on first-place bids, top-of-page bids, and first-page bids. So I made this script that will automate the CPC based on these three variables for all accounts and we can manage a spreadsheet.

I am a new programmer in JavaScript since a week, and it took me a week to build this script. I'd like to improve the structure. How could I modify my code in considering my request?

Is it possible to apply that strategy to my code to avoid the error:

Exceeded maximum execution time

Be aware that I put the related function runMe() in the code.

 function main() {
        adjustCPCmax();
}


String.prototype.format = function () {
  var i = 0, args = arguments;
  return this.replace(/{}/g, function () {
    return typeof args[i] != 'undefined' ? args[i++] : '';
  });
};


function isBlank(line) {
    return line[0].trim() === '' && line[1].trim() === '';
}


function parseData(data) {

    const output = {};
    var currentGroupName = '';

    data.forEach(function(line, index){
        if (isBlank(line) || index === 0){
            return; 
        }

        if (line[0].trim().length > 1) {
            currentGroupName = line[0].trim();
        }

        output[currentGroupName] = output[currentGroupName] || {};

        output[currentGroupName][line[1]] = line[2];
    });

    return output;
}


function runMe(fct) {
    var REASONABLE_TIME_TO_WAIT = 5;
    var startTime= (new Date()).getTime();

    fct

    var scriptProperties = PropertiesService.getScriptProperties();
    var startRow= scriptProperties.getProperty('start_row');
    for(var ii = startRow; ii <= size; ii++) {
        var currTime = (new Date()).getTime();
        if(currTime - startTime >= MAX_RUNNING_TIME) {
            scriptProperties.setProperty("start_row", ii);
            ScriptApp.newTrigger("runMe")
                .timeBased()
                .at(new Date(currTime+REASONABLE_TIME_TO_WAIT))
                .create();
            break;
        }
    }
}


function getMaxCPC(account, campaign) {
    //Your build Google-Spreadsheet
    var SPREADSHEET_URL = "https://docs.google.com/spreadsheets/d/1k4o_8O_11OvhZRergefWKgXQ8_XxIs7D31-NV9Ove-o/edit#gid=749396300";

    //Fetch and convert data in a JSON structure 
    var spreadsheet = SpreadsheetApp.openByUrl(SPREADSHEET_URL);
    var sheet = spreadsheet.getSheetByName('Campaigns');
    var data = sheet.getRange("A:C").getValues();
    var cleanData= parseData(data);

    //Formatting account and campaign
    var account = '{}'.format(account.getName())
    var campaign = '{}'.format(campaign.getName())

    Logger.log('Account :' + account)
    Logger.log('Campaign :' + campaign + '\n')

    try {
        return cleanData[account][campaign];
    }
    catch(e) {
    }
}


function adjustCPCmax() {
    //min CPC
    var minCPC = 0.50;

    var accountIterator = MccApp.accounts().get();
    var mccAccount = AdWordsApp.currentAccount();
    while(accountIterator.hasNext()) { 
        var account = accountIterator.next();
        Logger.log('============================================== ' + account.getName() + ' ====================================================')
        MccApp.select(account)

        var campaignIterator = AdWordsApp.campaigns().get();
        while (campaignIterator.hasNext()) {
            var campaign = campaignIterator.next();
            try {
                var maxCPC = getMaxCPC(account, campaign)
            }
            catch(e) {
            }

            if (maxCPC) {

                Logger.log('The entrence worked with max CPC : ' + maxCPC + '\n')
                keywordIterator = campaign.keywords().get();
                while (keywordIterator.hasNext()) {
                    var keyword= keywordIterator.next()
                    var keywordId = Number(keyword.getId()).toPrecision()

                    Logger.log('THE NAME OF THE KEYWORDID IS ' + keywordId + '\n')

                    var report = AdWordsApp.report(
                               'SELECT Id, Criteria, CampaignName, CpcBid, FirstPageCpc, FirstPositionCpc, TopOfPageCpc, Criteria ' +
                               'FROM   KEYWORDS_PERFORMANCE_REPORT ' +
                               'WHERE ' + 
                               'Id = ' + keywordId);
                    var rows = report.rows();
                    while(rows.hasNext()) {
                        var row = rows.next();
                        var keywordIdReport = row['Id'];
                        var keywordNameReport = row['Criteria'];
                        var campaignName = row['CampaignName'];
                        var cpcBid = row['CpcBid'];
                        var firstPageCpc = row['FirstPageCpc'];
                        var firstPositionCpc = row['FirstPositionCpc'];
                        var topOfPageCpc = row['TopOfPageCpc'];

                        Logger.log('BIG TEST')
                        Logger.log(keyword.getText())
                        Logger.log(keywordId)
                        Logger.log(keywordNameReport)
                        Logger.log(keywordIdReport + '\n')

                        if (keywordId === keywordIdReport) {
                            //if (firstPositionCpc <= maxCPC ) {
                                //var newCPC = firstPositionCpc;
                            //} else if (firstPositionCpc > maxCPC && topOfPageCpc <= maxCPC) {
                                //var newCPC = topOfPageCpc;
                            //} else if (topOfPageCpc > maxCPC && firstPageCpc <= maxCPC) {
                                //var newCPC = firstPageCpc;
                            //} else {
                                //var newCPC = minCPC;
                            //}

                            if (firstPositionCpc && (firstPositionCpc > 0 && firstPositionCpc <= maxCPC)) {
                                var newCPC = firstPositionCpc;
                            } else if (topOfPageCpc  && (topOfPageCpc > 0 && topOfPageCpc <= maxCPC)) {
                                var newCPC = topOfPageCpc;
                            } else if (firstPageCpc && (firstPageCpc > 0 && firstPageCpc <= maxCPC )) {
                                var newCPC = firstPageCpc;
                            } else {
                                var newCPC = minCPC;
                            }

                            Logger.log('KeywordIdReport :' + keywordIdReport)
                            Logger.log('campaignName :' + campaignName)
                            Logger.log('CPCbid :' + cpcBid)
                            Logger.log('firstPositionCpc : ' + firstPositionCpc)
                            Logger.log('topOfPageCpc : ' + topOfPageCpc)
                            Logger.log('firstPageCpc : ' + firstPageCpc)
                            Logger.log('NewCPC : ' + newCPC + '\n')

                            keyword.bidding().setCpc(newCPC)
                            break;
                        }
                    }
                }
            }
        }
    }
    MccApp.select(mccAccount);
}
\$\endgroup\$
1
\$\begingroup\$

It's hard for me to analyze this code because:

  • I don't know what CPC even means (I'm not in the Ads business).
  • The code using referencing modules from API I'm not familiar with.
  • I don't really understand what the code is trying to achieve.
  • It's a pretty big chunk of code and I'm not too keen on investing lots of time in it.

But even so, I can spot some obvious problems with this code. Starting from the top:

String.prototype.format()

String.prototype.format = function() {
    var i = 0, args = arguments;
    return this.replace(/{}/g, function () {
        return typeof args[i] != 'undefined' ? args[i++] : '';
    });
};

You're extending the native String object with format() method, only to use it for converting account and campaign names to string:

var account = '{}'.format(account.getName())
var campaign = '{}'.format(campaign.getName())

I would guess that account.getName() and campaign.getName() already return strings. But if that's not so, you could just cast them to string by simply: String(account.getName()).

isBlank()

function isBlank(line) {
    return line[0].trim() === '' && line[1].trim() === '';
}

I guess this is supposed to check if string consists only of whitespace characters. But why does it only check the first two characters? Or perhaps this line really isn't actually a line of text, but instead it's an array that contains two lines? The next function might tell us more...

parseData()

function parseData(data) {
    const output = {};
    var currentGroupName = '';

    data.forEach(function(line, index) {
        if (isBlank(line) || index === 0) {
            return;
        }

        if (line[0].trim().length > 1) {
            currentGroupName = line[0].trim();
        }

        output[currentGroupName] = output[currentGroupName] || {};

        output[currentGroupName][line[1]] = line[2];
    });

    return output;
}

Unfortunately the name parseData() doesn't tell us anything about what this function is doing. From getMaxCPC() that is using it I can understand that it generates a lookup map that's indexed by account and campaign name.

runMe()

function runMe(fct) {
    var REASONABLE_TIME_TO_WAIT = 5;
    var startTime = (new Date()).getTime();

    fct

    var scriptProperties = PropertiesService.getScriptProperties();
    var startRow = scriptProperties.getProperty('start_row');
    for (var ii = startRow; ii <= size; ii++) {
        var currTime = (new Date()).getTime();
        if (currTime - startTime >= MAX_RUNNING_TIME) {
            scriptProperties.setProperty("start_row", ii);
            ScriptApp.newTrigger("runMe")
                .timeBased()
                .at(new Date(currTime + REASONABLE_TIME_TO_WAIT))
                .create();
            break;
        }
    }
}

This function is just broken:

  • The fct parameter is used for...???
  • The size variable is not defined.
  • The MAX_RUNNING_TIME constant is also not defined.
  • Looks like the loop was pulled out from inside of some other loop... why else would you name the loop variable ii, not simply i?

getMaxCPC()

function getMaxCPC(account, campaign) {
    //Your build Google-Spreadsheet
    var SPREADSHEET_URL = "https://docs.google.com/spreadsheets/d/1k4o_8O_11OvhZRergefWKgXQ8_XxIs7D31-NV9Ove-o/edit#gid=749396300";

    //Fetch and convert data in a JSON structure
    var spreadsheet = SpreadsheetApp.openByUrl(SPREADSHEET_URL);
    var sheet = spreadsheet.getSheetByName('Campaigns');
    var data = sheet.getRange("A:C").getValues();
    var cleanData = parseData(data);

    //Formatting account and campaign
    var account = '{}'.format(account.getName())
    var campaign = '{}'.format(campaign.getName())

    Logger.log('Account :' + account)
    Logger.log('Campaign :' + campaign + '\n')

    try {
        return cleanData[account][campaign];
    }
    catch (e) {
    }
}

Reusing account and campaign parameters to store values of their respective names causes confusion. Just introduce new variables like accountName and campaignName so it'll be clear that these are different things from account and campaign parameters.

The final try-catch block looks questionable. You're trying to handle a scenario where account does not exist and so a lookup of campaign from undefined would result in error. A better approach would be:

return cleanData[account] && cleanData[account][campaign];

adjustCPCmax()

Now this is one beast of a function:

function adjustCPCmax() {
    //min CPC
    var minCPC = 0.50;

    var accountIterator = MccApp.accounts().get();
    var mccAccount = AdWordsApp.currentAccount();
    while (accountIterator.hasNext()) {
        var account = accountIterator.next();
        Logger.log('============================================== ' + account.getName() + ' ====================================================')
        MccApp.select(account)

        var campaignIterator = AdWordsApp.campaigns().get();
        while (campaignIterator.hasNext()) {
            var campaign = campaignIterator.next();
            try {
                var maxCPC = getMaxCPC(account, campaign)
            }
            catch (e) {
            }

            if (maxCPC) {

                Logger.log('The entrence worked with max CPC : ' + maxCPC + '\n')
                keywordIterator = campaign.keywords().get();
                while (keywordIterator.hasNext()) {
                    var keyword= keywordIterator.next()
                    var keywordId = Number(keyword.getId()).toPrecision()

                    Logger.log('THE NAME OF THE KEYWORDID IS ' + keywordId + '\n')

                    var report = AdWordsApp.report(
                        'SELECT Id, Criteria, CampaignName, CpcBid, FirstPageCpc, FirstPositionCpc, TopOfPageCpc, Criteria ' +
                        'FROM   KEYWORDS_PERFORMANCE_REPORT ' +
                        'WHERE ' +
                        'Id = ' + keywordId);
                    var rows = report.rows();
                    while (rows.hasNext()) {
                        var row = rows.next();
                        var keywordIdReport = row['Id'];
                        var keywordNameReport = row['Criteria'];
                        var campaignName = row['CampaignName'];
                        var cpcBid = row['CpcBid'];
                        var firstPageCpc = row['FirstPageCpc'];
                        var firstPositionCpc = row['FirstPositionCpc'];
                        var topOfPageCpc = row['TopOfPageCpc'];

                        Logger.log('BIG TEST')
                        Logger.log(keyword.getText())
                        Logger.log(keywordId)
                        Logger.log(keywordNameReport)
                        Logger.log(keywordIdReport + '\n')

                        if (keywordId === keywordIdReport) {
                            //if (firstPositionCpc <= maxCPC ) {
                                //var newCPC = firstPositionCpc;
                            //} else if (firstPositionCpc > maxCPC && topOfPageCpc <= maxCPC) {
                                //var newCPC = topOfPageCpc;
                            //} else if (topOfPageCpc > maxCPC && firstPageCpc <= maxCPC) {
                                //var newCPC = firstPageCpc;
                            //} else {
                                //var newCPC = minCPC;
                            //}

                            if (firstPositionCpc && (firstPositionCpc > 0 && firstPositionCpc <= maxCPC)) {
                                var newCPC = firstPositionCpc;
                            } else if (topOfPageCpc  && (topOfPageCpc > 0 && topOfPageCpc <= maxCPC)) {
                                var newCPC = topOfPageCpc;
                            } else if (firstPageCpc && (firstPageCpc > 0 && firstPageCpc <= maxCPC )) {
                                var newCPC = firstPageCpc;
                            } else {
                                var newCPC = minCPC;
                            }

                            Logger.log('KeywordIdReport :' + keywordIdReport)
                            Logger.log('campaignName :' + campaignName)
                            Logger.log('CPCbid :' + cpcBid)
                            Logger.log('firstPositionCpc : ' + firstPositionCpc)
                            Logger.log('topOfPageCpc : ' + topOfPageCpc)
                            Logger.log('firstPageCpc : ' + firstPageCpc)
                            Logger.log('NewCPC : ' + newCPC + '\n')

                            keyword.bidding().setCpc(newCPC)
                            break;
                        }
                    }
                }
            }
        }
    }
    MccApp.select(mccAccount);
}
  • It's just way too big and very deeply nested. It should be split up to smaller functions.
  • There's a lots of logging. Is logging really the purpose of this function?
  • There's commented-out code. Delete it instead.
  • There's additional try-catch block around getMaxCPC() which already has a try-catch block inside it. One should be enough.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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