blasee/fetchR — pull request #1 by sjp (2014-08-28 11:22:16+00:00)
Title: Cleanup and other various changes

Hi Blake -- another package another pull request.

Aside from adding in the usual Travis CI support, I've ensured that the fetchR now passes the package checker. In the process I've dropped the dependency on `rgeos`, as it didn't seem to be used at all.

I've also had a go had a code review. Mostly this involved the following:

* Extracting a variable to avoid repeated calculation (multiple `min()` and `max()` calls)
* Using `dev.hold()` and `dev.flush()` to speed up plot drawing -- it's probably fast enough anyway.
* Dropping `isTRUE()`. I've added an `NA` check prior to this so we can safely use `if (plot)` for example.
* Using `anyNA()`, should be faster, but mostly it's just easier to read.
* Minor formatting changes.

These changes have not been tested by myself so I can't guarantee whether they work, but they are at least syntactically correct.

A few nit-picky thoughts as an aside:

* How about something other than PNG plotting? I'm thinking PDF in particular. Might be worth separating the plotting out into a separate function (maybe using S3?) to do this for you.
* I don't know how clean your data is, but it might be worth guarding against NA or zero-length vectors. There were some places in your code where that code happen -- but it seems unlikely because you would already have run into it if that were the case.
* Exposing your data via `data()`. I am guessing you want to keep the data internal to the package (reasonable).

Congrats on yet another package! Keep up the good work!