BenFradet / RiotSharp

C# wrapper for the Riot Games API
http://benfradet.github.io/RiotSharp/
MIT License
300 stars 145 forks source link

Fixing static data values #371

Closed JanOuborny closed 6 years ago

JanOuborny commented 7 years ago

Since the static data endpoint provides a lot of wrong or bad data (which is still persistent in v3). We should provide a function to fix all of this data. Riot is also recommending this solution by themselves. To start this we should start listing all know issues with the static api.

Known issues:

BenFradet commented 7 years ago

I don't know about fixing the results ourselves, this doesn't seem like a good solution in the long-term (a lot of maintenance).

JanOuborny commented 7 years ago

I know that it comes with a lot of maintenance, therefore I wouldn't integrated completely into the api endpoint. I would rather offer an option for it. So that even if we are behind in updating the code, it doesn't break the whole endpoint.

BenFradet commented 7 years ago

I hear what you're saying however I personally think RiotSharp is meant to be "a dumb wrapper" ie people should expect the same things when using RiotSharp or hitting the API directly.

toothlessG22 commented 7 years ago

I'm not sure if this would be feasible, but what if you allowed the user to add the missing data. Say you had a function that the user could access called overrideChampionStaticData(championId, championDto). You store that championDto so you can return it whenever the user calls for that specific id. I don't know how the caching feature works, but you might be able to leverage that by storing this championDto in the cache.

JanOuborny commented 7 years ago

In this case you could make the static api methods virtual and let the users do whatever they want with them. However, I like your idea, but I would take a more generic approach. I would provide a method RegisterDataProvider(IDataProvider<TEntity> provider) which registers a provider for the corrected data. This provider gets then called when populating the cache.

JanOuborny commented 6 years ago

Closed because of #564