paquettg / leaguewrap

League of Legend API wrapper
MIT License
67 stars 28 forks source link

[Discussion] Loading static data for dto objects #41

Closed paquettg closed 9 years ago

paquettg commented 10 years ago

Hey, I wanted to pose this question to anyone interested in chiming in. I am currently adding a feature to the 0.6.0 branch that allows developers to automatically load static data in response DTO. So, for example, if you make a call to the champion endpoint you will get a response similar to

.array(7) {
  'id' =>
  int(10)
  'active' =>
  bool(true)
  'botEnabled' =>
  bool(true)
  'freeToPlay' =>
  bool(false)
  'botMmEnabled' =>
  bool(true)
  'rankedPlayEnabled' =>
  bool(true)
  'championStaticData' =>
  array(4) {
    'id' =>
    int(10)
    'key' =>
    string(5) "Kayle"
    'name' =>
    string(5) "Kayle"
    'title' =>
    string(13) "The Judicator"
  }
}

The static data is already loaded into the response. The question is how should the programmer be able to activate this feature? At the moment I have 2 ideas on how to implement this (they can both be implemented, they are not mutually exclusive) you can do one of the following.

$api = new Api(...);
$api->attachStaticData();
$kayle = $api->champion()->championById(10);

or

$api = new Api(...);
$kayle = $api->champion()->championById(10);
$kayle->loadStaticData($api->staticData());

Which one do you prefer? Do you have another idea on how this would be better implemented? Any feedback is welcome.

dnlbauer commented 10 years ago

I like the idea since it makes it a lot easier to merge static data into nonstatic without keeping a reference to the static data on my own.

I'd prefer the first syntax. The second one looks a bit awkward for me. There you have to pass api objects arround. The first one is a single call to get everything ready. Nice and easy.

paquettg commented 10 years ago

This has been implemented in the new version. Let me know what you think :)

dnlbauer commented 9 years ago

I like it :)