MySportsFeeds / mysportsfeeds-api

Feature requests for the MySportsFeeds Sports Data API.
44 stars 3 forks source link

Field Naming in JSON API (part duex) #87

Closed thegreatco closed 6 years ago

thegreatco commented 6 years ago

I was just poking around the beta docs since you closed some of my previous reports (Thanks for putting those updates in!) and I'm not sure if the docs just haven't been updated but there is an odd naming convention being used for schedules and other lists of games, specifically

{
    "games": {
        "lastUpdatedOn": ISODate(),
        "game": [ {
            // some game object
        },
        {
            // some game object
        }]
        "references": { } // This contains reference data
}

This would mean accessing an actual game is games.game[x]. This means the plural "games" does not actually contain a "list" of game. And the singular game contains a "list" of game. While this makes more sense in the xml version of the API, in the json version it doesn't quite fit.

A more apt naming scheme in JSON might be

// GET on https://api.mysportsfeeds.com/v2.0/pull/mlb/2018-regular/date/20180411/games.json
{
    "lastUpdatedOn": ISODate(),
    "games": [ {
        // some game object
    },
    {
        // some game object
    }]
    "references": { } // This contains reference data
}

Which makes accessing not just a single game a bit more intuitive

var game = schedule.games[0];

But it also makes more sense as the API now returns a Schedule (or whatever you want to call it) object, not an object wrapping the schedule data.

This also makes entity naming for de-serializing this API into strongly typed objects make a bit more sense. For example, in C#, based on the current beta docs

public class Games
{
    public DateTime LastUpdatedOn {get;set;}
    public List<Game> Game {get;set;}
    public References References {get;set;}
}

This whole object also needs to be wrapped in another class because the API response is not of type Games it is of a type that contains Games. So I would need something like

public class GamesResponse
{
    public Games {get;set;}
}
bradbarkhouse commented 6 years ago

This is a side-effect of having to support the XML format, using the same metadata. As you noted, XML requires an enclosing element, which is what the "games" represents - there's no easy way to remove that right now in our feed engine. Perhaps though, it could be renamed to something like "seasonalGames", "dailyGames" or "weeklyGames" relating to the feed's name? Would that be a decent compromise?

bradbarkhouse commented 6 years ago

@thegreatco Actually scratch that last comment, I'm going to look further into the possibility of dropping the enclosing JSON property. And then renaming "game" to "games" is trivial. :)

bradbarkhouse commented 6 years ago

Great news @thegreatco ! Looks like this is easier than I thought - we'll basically just strip those tags out before we generate the JSON. Should have that change in soon ... then I'm gonna have to go back and modify all the sample responses as well. :)

bradbarkhouse commented 6 years ago

Done!

thegreatco commented 6 years ago

Hey @bradbarkhouse thanks for the quick response! Happy to hear its been updated. Sorry I was AFK for the weekend but I just checked it out and this looks great!