5
\$\begingroup\$

I've developed this spreadsheet in order to scrape a website's number of indexed pages through Google and Google Spreadsheets.

I'm not a developer, so how can I improve this code in order to have less code, to use less resources, or to go faster?

I've explained everything here.

//-------------------------------------------
//   scrape a website's number of indexed pages through Google and Google Spreadsheets.
//-------------------------------------------
function indexedpages(myUrl)
    {   
    var request = "http://www.google.com/search?&hl=en&q=site:http://" + unescape(myUrl); // Google Request for indexed pages

    var sourcecode = UrlFetchApp.fetch(request).getContentText(); // scrape the page content

    Utilities.sleep(1000); // 1000ms pause to prevent spam

    var codebefore = '<div id="resultStats">About '; // the code before the number we want

    var codeafter = ' results<'; // the code after the number we want

    var theresult = sourcecode.substring(sourcecode.indexOf(codebefore)+codebefore.length, sourcecode.indexOf(codeafter)); // scrape the number we want

    var theresult =  theresult.replace(",", ""); // delete the ","

    if (isNaN(theresult)) // if the result is not a number

    {        
        var codebefore = '<div id="resultStats">'; // the code before the number we want

        var codeafter = ' result<'; // the code after the number we want

        var theresult = sourcecode.substring(sourcecode.indexOf(codebefore)+codebefore.length, sourcecode.indexOf(codeafter)); // scrape the number we want           

        if (isNaN(theresult)) // if the result is not a number

        {
            var codebefore = '<div id="resultStats">'; // the code before the number we want

            var codeafter = ' results<'; // the code after the number we want

            var theresult = sourcecode.substring(sourcecode.indexOf(codebefore)+codebefore.length, sourcecode.indexOf(codeafter)); // scrape the number we want  

            if (isNaN(theresult)) // if the result is not a number (0 result)

            {          
                return 0; // returns 0
            }

            else

            {
                return theresult; // returns the number we want
            }                            
        }

        else 
        {
            return theresult; // returns the number we want
        }

    }

    else 
    {
        return theresult; // returns the number we want
    }

}
\$\endgroup\$
  • \$\begingroup\$ I'm afraid somebody who doesn't speak French can't understand your question as the blog, the variable names and the comments are all in French. You should at least translate the variables and comments for this question here. \$\endgroup\$ – Denys Séguret Jul 22 '13 at 9:14
  • 2
    \$\begingroup\$ I just translated it ;) \$\endgroup\$ – Kuhm Cyril Jul 22 '13 at 9:43
  • \$\begingroup\$ Good. Especially useful for the variables. This one could probably not win the prize for the most useful comment ever : // fonction name. \$\endgroup\$ – Denys Séguret Jul 22 '13 at 9:45
  • \$\begingroup\$ Of course ^^ I added all the comments for my blog visitors, because some of them really don't know nothing about developement. \$\endgroup\$ – Kuhm Cyril Jul 22 '13 at 11:15
1
\$\begingroup\$

Use a regular expression to look for the relevant text in the page.

It makes more sense to return a number instead of a string.

In the parameter name, "my…" is pointless. Just call the parameter site.

Google now encourages the use of HTTPS everywhere, so it's probably a good idea to scrape Google using HTTPS as well. Also, Google doesn't need the site:... query argument to start with "http://". Finally, the correct way to build a query string is to use encodeURIComponent().

function indexedpages(site) {
    // Google request for relevant pages
    var request = "https://www.google.com/search?&hl=en&q=site:" + encodeURIComponent(site);
    // scrape the page content
    var sourcecode = UrlFetchApp.fetch(request).getContentText();
    // 1000ms pause for rate limiting
    Utilities.sleep(1000);

    var match = /<div id="resultStats">(?:About )?([0-9,]*) results?</.exec(sourcecode);
    return (match) ? parseInt(match[1].replace(',', '', 'g')) : 0;
}
\$\endgroup\$
  • \$\begingroup\$ Thanks a lot ! Your code is great. Just saw an error with a site having more than 1000000 pages. Could you check ? \$\endgroup\$ – Kuhm Cyril Oct 21 '13 at 13:05
  • 1
    \$\begingroup\$ Fixed with the g flag when calling replace(). (The bug was propagated from your original code.) \$\endgroup\$ – 200_success Oct 21 '13 at 13:20
  • \$\begingroup\$ You are a boss ! :) \$\endgroup\$ – Kuhm Cyril Oct 24 '13 at 11:57
1
\$\begingroup\$

Comments

  • Never place them between the closing parenthesis and the opening bracket of the statement.

  • Comments are meant to be read, and not set aside. Here are suggestions for cleaner comments:

    //Before the lines concerned, like this one
    someObject.someFunction();
    
    //Inside blocks of statements
    if(foo){
      //If foo, then call foo
      foo();
    } else if(bar){
      //If bar, then call bar
      bar();
    }
    

Prettify Code

To make your code a bit readable, but save yourself the hassle of formatting it manually, use a prettifier. An example is jsbeautifier.

Code

function indexedpages(myUrl) {

  // JavaScript doesn't have block scope like C. Variables declared in blocks
  // other than functions are pulled up and declared in the nearest scope.
  var i;
  var codeBefore;
  var codeAfter;
  var result;

  // So I assume your code was about looking for numbers between a certain set
  // of strings. It's a repetetive task, we'll use an array to store these set
  // of strings and a loop that accesses them using an index.
  var wrappers = [
    ['<div id="resultStats">About ', ' results<'],
    ['<div id="resultStats">', ' result<'],
    ['<div id="resultStats">', ' results<']
    //add more wrappers if you want
  ];

  var wrappersLength = wrappers.length;

  // Make the request
  var request = "http://www.google.com/search?&hl=en&q=site:http://" + unescape(myUrl);
  var sourceCode = UrlFetchApp.fetch(request).getContentText();

  Utilities.sleep(1000);

  // Then loop through each of the wrappers and testing them
  for (i = 0; i < wrappersLength; ++i) {

    // Get the codeBefore and codeAfter from an entry in the array
    codeBefore = wrappers[i][0];
    codeAfter = wrappers[i][1];

    // Parse
    result = sourceCode.substring(sourceCode.indexOf(codeBefore) + codeBefore.length, sourceCode.indexOf(codeAfter)).replace(",", "");

    // Then check if the number is not NaN and if so, return which effectively
    // make the code break out of the loop and of the function as well.
    if (!isNaN(result)) return result;
    else return "0";    
  }
}

When free of comments, it's just this short:

function indexedpages(myUrl) {
  var i;
  var codeBefore;
  var codeAfter;
  var result;
  var wrappers = [
    ['<div id="resultStats">About ', ' results<'],
    ['<div id="resultStats">', ' result<'],
    ['<div id="resultStats">', ' results<']
  ];
  var wrappersLength = wrappers.length;
  var request = "http://www.google.com/search?&hl=en&q=site:http://" + unescape(myUrl);
  var sourceCode = UrlFetchApp.fetch(request).getContentText();
  Utilities.sleep(1000);
  for (i = 0; i < wrappersLength; ++i) {
    codeBefore = wrappers[i][0];
    codeAfter = wrappers[i][1];
    result = sourceCode.substring(sourceCode.indexOf(codeBefore) + codeBefore.length, sourceCode.indexOf(codeAfter)).replace(",", "");
    if (!isNaN(result)) return result;
    else return "0"
  }
}
\$\endgroup\$
  • \$\begingroup\$ Thanks a lot ! I just edited your code, i found 2 mistakes and added the "0" return. \$\endgroup\$ – Kuhm Cyril Jul 22 '13 at 17:19

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.