Design Thoughts
edwardloveall opened this issue · 5 comments
Here's the interface I had in mind:
Stattleship.fetch(
sport: 'hockey',
league: 'nhl',
action: 'stats',
filters: {
type: 'hockey_defensive_stat',
stat: 'hits'
}
)
Why Strings?
TL;DR: Less string-to-method conversions.
Consider the more object/method approach:
Stattleship::Hockey.nhl.stats(type: 'hockey_defensive_stat', stat: 'hits')
If user is filling out some form to get stats, the programmer will need to do things like Stattleship::Hockey.send(params[:league])
which uses send
which is not ideal. Specifying the query out in a hash is much easier to compose.
And alternative approach is using methods as the categories with arguments:
Stattleship::Hockey.league('nhl').
action('stats').
filters(
type: 'hockey_defensive_stat',
stat: 'hits')
While this is better than the fully method-ized option, it does require more maintenance on Stattleship's part as new methods have to be added or removed for different types of queries. Maybe these aren't added that often.
Error checking
Also, while this is a somewhat ambitious goal, the objects themselves don't necessarily need any logic in them and be pure descriptions of the objects. This can allow for data integrity like disallowing values that can't defined in the model. For example:
Stattleship.fetch(
sport: 'hockey',
league: 'nhl',
action: 'stats',
filters: {
type: 'foo',
stat: 'bar'
}
)
#=> Stattleship::UnrecognizedFilterError: We don't recognize the filter values type: 'foo' or stat: 'bar'
I also was thinking of being able to do
Stattleship::Hockey.nhl.stats(type: 'hockey_defensive_stat', stat: 'hits')
effectively the "action" here of "stats" maps to an endpoint (stats, games, feats, teams, players, and so forth).
Each of those action/endpoint methods would call some lower-level fetch that would know that "nhl" needs to set the sport to "hockey" and "league" to nhl.
Sure, we'd have to update the gem when we support new leagues, but I think that's appropriate. And, it prevent people from attempting to get stats for sports or leagues that we don't support (golf, cfl, etc).
I also considered having a scoping like:
Stattleship::Hockey.nhl.stats.for_stat(type: 'hockey_defensive_stat', stat: 'hits')
so that could chain
Stattleship::Hockey.nhl.stats.
for_team('nhl-bos').
since(2.days.ago).
for_stat(type: 'hockey_defensive_stat', stat: 'hits')
where these would make a query string of:
team_id = nfl-bos
since = 2 days ago
type = hockey_defensive_stat
stat = hits
or
since = <some-epoch-timestamp>
The game, stats, and feats filters will support since, at, on
time filters in next release (14-december).
I was also toying with being able to alias methods such that
for_team('nhl-bos')
can be
bruins
or boston-bruins
in the Hockey namespace.
There would be ~ 30-32 * flavor of these team aliases per league.
But, that's a nice to have.
Might it also be
Stattleship::Hockey::Nhl::Stats.for_stat(type: 'hockey_defensive_stat', stat: 'hits')
?
It's defiantly the more traditional setup to have the ruby constants or method chaining. My worry is the inflexibility of this. This works great if you know exactly the stat you want before it's requested, i.e. for this thing, I always want hockey stats from the NHL. But what if you don't? If you start making more abstract queries this breaks down.
Like if you have rails params:
{
"controller" => "sports_queries",
"action" => "show",
"sports_query": {
"sport" => "hockey",
"league" => "nhl",
"action" => "stats",
"filters" => {
"type" => "hockey_defensive_stat",
"stat" => "hits"
}
}
}
Now you have to convert all those strings. In writing this out though, I do realize that it's probably not the main use case and it's more common to know what you're requesting. I'm still a fan of it because I see little downside to my proposed method, where is I do see a downside, even if it's not large, to using ruby constants or method chains.
You make a compelling point, given using the gem in a Rails controller.
While it is true that 95th percentile you'd know exactly what you are requesting, let's implement the "fetch" or perhaps "paginate" (to use Octokit's naming) for a generic endpoint.
We can refactor to implement specific named methods if we find that necessary.
In any event, the "low-level" paginate request/response is needed first.