jprichardson / node-google

A Node.js module to search and scrape Google.
MIT License
454 stars 115 forks source link

added optional start parameter, enabling pagination #29

Closed ashack293 closed 9 years ago

ashack293 commented 9 years ago

This pull requests adds an optional starting index for google requests. You can set the number of results by which you want your query to be offset. To use it, change syntax from:

google ('node.js best practices',  callback) 

to:

google ('node.js best practices',  10, callback) 
ashack293 commented 9 years ago

Is there any way to rerun the Travis build? I'm not sure that the failure was related to my changes.

ashack293 commented 9 years ago

Here are my local test results:

 + google()
    ✓ should return search results (1405ms)
    when resultsPerPage is set
      ✓ should return search results (625ms)
    when timeSpan is set
      ✓ each time-based query should return search results (463ms)
    when nextText and lang are set
      ✓ should return next page search results (1694ms)

  4 passing (4s)
jprichardson commented 9 years ago

Paging is already supported. What's the use case for not wanting to start on the first page?

ashack293 commented 9 years ago

My use-case is an application running on multiple instances which needs to add a batch of results to a database independent of the state of other instances, and independent of the application lifecycle on a single instance.

Making the paging an optional parameter with a few lines of code made a lot more sense to me than having to build state management around the module. This change makes it easy to check a database for the index of the last page retrieved from Google, and run the next query from there.

As an aside, it is easier to use the module without abusing it if you do a deeper (multi-page) search over a longer period of time instead of paging on a loop.

jprichardson commented 9 years ago

As an aside, it is easier to use the module without abusing it if you do a deeper (multi-page) search over a longer period of time instead of paging on a loop.

This is an excellent point. Ok, I'll incorporate this... I think that I wanna refactor the module...

jprichardson commented 9 years ago

I'll accept the PR, but first, please add a test. Something as simple as searching for something with the new parameter and checking for results.

ashack293 commented 9 years ago

Great! Test added.

jprichardson commented 9 years ago

Thanks, published :)