Add season param to fotmob_get_league_[matches|tables]
#256
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #256 +/- ##
==========================================
+ Coverage 67.31% 67.49% +0.18%
==========================================
Files 49 49
Lines 4081 4113 +32
==========================================
+ Hits 2747 2776 +29
- Misses 1334 1337 +3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| url <- httr::parse_url("https://www.fotmob.com/api/leagues") | ||
| url$query <- list( | ||
| "id" = league_id, | ||
| "season" = season | ||
| ) | ||
| url <- httr::build_url(url) |
There was a problem hiding this comment.
building the URL like this is advantageous since we may not have a value for season. build_url() knows to ignore a NULL
| # Need to coerce to `NA_character` since crossing doesn't like `NULL` | ||
| season <- ifelse(is.null(season), NA_character_, season) |
There was a problem hiding this comment.
i much prefer to use NULL for the absence of an optional argument, but because crossing() will drop NULLs, i temporarily change NULL to NA_character_
…m/JaseZiv/worldfootballR into improve-fotmob_get_league_matches
| resp <- .fotmob_get_league_resp(league_id, page_url) | ||
| .fotmob_get_league_matches <- function(league_id, page_url, season = NULL) { | ||
| # And now coerce NA back to NULL | ||
| season <- switch(!is.na(season), season, NULL) |
There was a problem hiding this comment.
have to use switch() instead of if or ifelse() since those suck with handling NULLs
|
Really nice - I'm getting a few learnings out of this PR which is an added bonus! Thanks for this update. |
Fixes #250. (
fotmob_get_league_tables()has a similar backend tofotmob_get_league_matches(), so I addressed it at the same time.)