avivbiton / BlizzardApiReader

.Net Core library to handle Blizzard public APIs
MIT License
35 stars 36 forks source link

Added remaining Diablo 3 API requests #37

Closed oliverkovac closed 5 years ago

oliverkovac commented 5 years ago

Addressing issue #14

Hello guys! Please take a look at the object structures and let me know if it's acceptable this way, or if you have a better idea of organizing it.

avivbiton commented 5 years ago

Hey. Good job. Did you test these models? I'll take a closer look today when I am home and merge if anything is good

oliverkovac commented 5 years ago

Yup I tested them - also thought about making some mock JSON responses to enable automated testing of the parsing/models, what do you think about that?

Couldn't find any official full JSON structure from Blizzard though, and they got a null ignore policy when constructing the JSONs, so I'm not sure if the C# object model includes all the possible fields, but I tested the calls on multiple end-game characters to have non-null values in the maximum possible amount of fields, so I hope I got it covered. If you know a better approach let me know, I can have another look at it.

I'm also a bit unsure about the naming, I put Profile in front of everything so it doesn't collide with current existing models. Some of the new ones were almost the same, but these Profile objects often had some more data in them, so I rather created new models for everything from that API section. I was thinking about making a different namespace (i.e. BlizzardApiReader.Diablo.Models.Profile) for them so the names are nicer, but didn't want to do that without asking first.

avivbiton commented 5 years ago

Beside, holding more data, is there any other difference to them?
How are they different from the normal models ? If we make an entire namespace for them, I don't want to make this confusing.
I had an idea to add a Suffix to them, like "Extended", so it will be very obvious what is the difference between the one, what do you think?

oliverkovac commented 5 years ago

Nah, not really - the only difference is, all of these refer to some existing concrete player's items, followers etc at a given moment, so they contain "dynamic" data - with the other API requests you always get the same data for the same request (the starting API path is also a bit different - /d3/profile/... vs /d3/data/...). But the models are still just models.

Sounds good, I will rename them then! Just gimme a while, I'm still a bit clunky with git, I usually work with Mercurial and I don't wanna screw this up:)

avivbiton commented 5 years ago

Thanks. Merged. Please feel free to open an issue if you have a suggestion or anything else.