amrrs / coinmarketcapr

💰R package to get Cryptocurrencies Market Cap Prices from Coin Market Cap 💰
https://amrrs.github.io/coinmarketcapr/
Other
81 stars 21 forks source link

respect further parameters, especially 'limit', if latest is TRUE #23

Closed mabe0033 closed 3 years ago

mabe0033 commented 4 years ago

To save API credits, it would be nice to be able to call 'get_crypto_listings' with parameter 'limit=200' even if latest is TRUE, which is easily achieved with the tiny modifications in this commit/request.

amrrs commented 4 years ago

Thanks for the PR @mabe0033 , Will look into it!

anagumang commented 3 years ago

The proposed commit works, but the 4 suggested lines of code are redundant. Instead of this pull request, re-arranging the code of the original function is fully sufficient:

diff --git a/R/currencies.R b/R/currencies.R
index f28f255..c149482 100644
--- a/R/currencies.R
+++ b/R/currencies.R
@@ -153,19 +153,19 @@ get_crypto_listings <- function(currency = "USD", latest = TRUE, ...) {
     base_url <- .get_baseurl()

     ## Build Request (new API) ##########
+    whatelse <- list(...)
     if (latest) {
         what <- paste0("cryptocurrency/listings/latest?convert=", currency)
     } else {
         what <- paste0("cryptocurrency/listings/historical?convert=", currency)
-        whatelse <- list(...)
         if (!"date" %in% names(whatelse)) {
             stop(cat(crayon::red(cli::symbol$cross,
                                  "A 'date' argument is needed for historical data.\n")))
         }
-        whatelse <- transform_args(whatelse)
-        if (!is.null(whatelse))
-            what <- paste0(what, "&", whatelse)
     }
+    whatelse <- transform_args(whatelse)
+    if (!is.null(whatelse))
+        what <- paste0(what, "&", whatelse)

     apiurl <- sprintf("https://%s/v1/%s", base_url, what)
amrrs commented 3 years ago

Thanks very much you both! I've implemented as @anagumang suggested 💯 !