CODEWITHSUNDEEP
javascriptgoogle-chromegoogle-chrome-extensioncoding-styleglobal-variables
user4061059
user4061059

Reputation:

Javascript: global variables = bad?

I have made a simple Chrome extension to automatically take me to the next episode of a series i'm watching. Just to be clear: the code WORKS! :p

But, since calls to the Chrome API are mostly asynchronous, I had to cut the function in 3 different parts and I had to use a global variable to be able to open a new tab.

Since I have always heard that global variables are bad style, I would like to know what's possible to solve this problem (albeit it a minor one).

Any tips are welcome!

//Global var = bad style?
var newUrl;

//Event listener for extension button click
chrome.browserAction.onClicked.addListener(getHistory);

function getHistory (tab) {
    chrome.history.search({text: "watchop.com/watch/"}, processHistory)
}

function processHistory  (history) {
    var lastUrl = history[0].url;
    var regEx = /-(\d{3})-/;

    //There is no history or the last page was the home page, so just go to the home page
    if ( (history.length == 0)  || !regEx.test(lastUrl)) {
        newUrl = "http://www1.watchop.com/";

    //There is history: get the number of the last viewed episode
    } else {
        //grab first captured group
        var lastEp = regEx.exec(lastUrl);
        lastEp = lastEp[1];
        var newEp = parseInt(lastEp) + 1;
        newUrl = "http://www1.watchop.com/watch/one-piece-episode-" + newEp + "-english-subbed/";
    }

    //Get possible tabs in which OP is opened
    chrome.tabs.query({url: "http://www1.watchop.com/*"}, gotoOP);
}

function gotoOP (tabs) {

    //There are open tabs
    if (tabs.length != 0) {
        var tab = tabs[0];
        chrome.tabs.highlight({tabs: tab.index}, doNothing);
        //Change window location of the active tab
        chrome.tabs.executeScript(tab.id, {code: "document.location = '" + newUrl + "'"}, doNothing)

    //No open tabs, just make a new one
    } else {
        chrome.tabs.create({url: newUrl})
    }
}

//Bogus function because some chrome API calls require a callback function
function doNothing (window) {
    return;
}

Upvotes: 2

Views: 610

Answers (2)

Xan
Xan

Reputation: 77531

You can use higher-order functions (functions that return functions) to construct "custom" callbacks that conform to signatures Chrome API expects.

Your problem is: you want a callback to the chrome.tabs.query function with an additional parameter newUrl, while the API will only supply the tab list to the callback.

Solution: write a parametrized callback

function gotoOP(url) {
  return function(tabs) { // <-- Construct and return an (anonymous) function
    //There are open tabs
    if (tabs.length != 0) {
      var tab = tabs[0];
      chrome.tabs.highlight({tabs: tab.index}, doNothing);
      //Change window location of the active tab
      chrome.tabs.executeScript(
        tab.id,
        {code: "document.location = '" + newUrl + "'"},
        doNothing
      );
    //No open tabs, just make a new one
    } else {
      chrome.tabs.create({url: url}); // <-- Using the parameter here
    }
  };
}

//Get possible tabs in which OP is opened
chrome.tabs.query({url: "http://www1.watchop.com/*"}, gotoOP(newUrl));

In fact, JavaScript has a function called bind(), that allows you to set what this will be when the function is called. You can use it too:

function gotoOP(tabs) {
  //There are open tabs
  if (tabs.length != 0) {
    var tab = tabs[0];
    chrome.tabs.highlight({tabs: tab.index}, doNothing);
    //Change window location of the active tab
    chrome.tabs.executeScript(
      tab.id,
      {code: "document.location = '" + newUrl + "'"},
      doNothing
    );
  //No open tabs, just make a new one
  } else {
    chrome.tabs.create({url: this.url}); // <-- Using the parameter here
  }
}

chrome.tabs.query({url: "http://www1.watchop.com/*"}, gotoOP.bind({url: newUrl}));

Upvotes: 1

Beri
Beri

Reputation: 11620

Yest this is true, because in JS variables and functions have scope the context in witch they are defined. In your code you have 1 variable and 4 global functions. To hide them you need to wrap them with some kind of funtion block, either named or not.

  1. You can wrap it with self executing JS function, that way iy will hide all logic, and will not polute global namespace:

(function() {
    alert('Hello World');
})();

http://esbueno.noahstokes.com/post/77292606977/self-executing-anonymous-functions-or-how-to-write

  1. (same as first but executes after dom loads): You could place this code inside a ready block (involving jQuery), it will give same result.

  2. wrap whole function in a class: http://www.phpied.com/3-ways-to-define-a-javascript-class/

Upvotes: 0

Related Questions