The response serializations are insane
Morhyn.8032:
Why do you insist on including the method name in the response? As an example, if we query /v1/continents.json we know we’re getting back a list of continents. We don’t need to be told again. The serialization would be much simpler like so:
{
"1": {
"name": "Tyria",
"continent_dims": [ 32768, 32768 ],
"min_zoom": 0,
"max_zoom": 7,
"floors": [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17,
18, 19, 30, -2, -3, -4, -5, -6, -7, -8, -10, -11, -15, -16, -17, 38,
20, 21, 22, 23, 24, 25, 26, 27, 34, 36, 37 ]
},
"2": {
"name": "Mists",
"continent_dims": [ 16384, 16384 ],
"min_zoom": 0,
"max_zoom": 6,
"floors": [ 1, 3, 5, 6, 7, 8, 9, 10, 13, 14, 18, 19, 21, 22, 23, 24, 25,
26, -27, -28, -29, -30, -31, -32, -33, 27 ]
}
}
In fact, I’d much rather see:
[
{
"id": 1,
"name": "Tyria",
"continent_dims": [ 32768, 32768 ],
"min_zoom": 0,
"max_zoom": 7,
"floors": [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17,
18, 19, 30, -2, -3, -4, -5, -6, -7, -8, -10, -11, -15, -16, -17, 38,
20, 21, 22, 23, 24, 25, 26, 27, 34, 36, 37 ]
},
{
"id": 2,
"name": "Mists",
"continent_dims": [ 16384, 16384 ],
"min_zoom": 0,
"max_zoom": 6,
"floors": [ 1, 3, 5, 6, 7, 8, 9, 10, 13, 14, 18, 19, 21, 22, 23, 24, 25,
26, -27, -28, -29, -30, -31, -32, -33, 27 ]
}
]
Ryan.9387:
The brackets would be easier, but I don’t have any trouble just using json parse to make it into a dictionary.
The headers are useful in their own way. Maybe not for your call. But some provide totals and such. (If I’m interpreting your complaint correctly.)
Morhyn.8032:
Where do the “headers” provide a total?
Ryan.9387:
BLTP is the first that comes to mind. You can search with count=0 and get a total count returned in a part of the json.
Morhyn.8032:
I’m sorry, I don’t know what “BLTP” is an acronym for. I don’t see anything in the list that it could be — http://wiki.guildwars2.com/wiki/API:1
smiley.1438:
I’m sorry, I don’t know what “BLTP” is an acronym for. I don’t see anything in the list that it could be — http://wiki.guildwars2.com/wiki/API:1
BLTP – Black Lion Trading Post (my guess)
DarkSpirit.7046:
Let me try to answer this in the context of C#. For the first case, it is natural to deserialize to a Dictionary and for the second, a List.
In terms of lookup times, the Dictionary is usually faster than the List, unless you have only 3 items or fewer.
http://www.dotnetperls.com/dictionary-time
Morhyn.8032:
Let me try to answer this in the context of C#. For the first case, it is natural to deserialize to a Dictionary and for the second, a List.
In terms of lookup times, the Dictionary is usually faster than the List, unless you have only 3 items or fewer.
Yes, if you’re deserializing my suggested “fixed” serializations. With the actual API result you either:
- deserialize to a dictionary with one key who’s value is the real result
- write a custom deserializer to ignore the useless outer key
I don’t see the point in either case. Not when the API could simply return the real result very easily.
Morhyn.8032:
I’m sorry, I don’t know what “BLTP” is an acronym for. I don’t see anything in the list that it could be — http://wiki.guildwars2.com/wiki/API:1
BLTP – Black Lion Trading Post (my guess)
Unless I’ve missed it, that isn’t a part of the official public API.
smiley.1438:
I’m sorry, I don’t know what “BLTP” is an acronym for. I don’t see anything in the list that it could be — http://wiki.guildwars2.com/wiki/API:1
BLTP – Black Lion Trading Post (my guess)
Unless I’ve missed it, that isn’t a part of the official public API.
It isn’t – but as you can read in this subforum, it’s happily used…
Morhyn.8032:
I’m sorry, I don’t know what “BLTP” is an acronym for. I don’t see anything in the list that it could be — http://wiki.guildwars2.com/wiki/API:1
BLTP – Black Lion Trading Post (my guess)
Unless I’ve missed it, that isn’t a part of the official public API.
It isn’t – but as you can read in this subforum, it’s happily used…
Okay, well let me clarify: I’m only concerned with what is published as “officially public.” I haven’t seen anything in that API where dictionary keys are used as a total for a search result. What I have repeatedly seen is the name of the API method being returned as an initial dictionary key with no explained reason. Which, as I mentioned in a previous reply, is annoying during deserialization due to having to filter it out or write weird objects to map the result to.
smiley.1438:
Just btw. – We’re talking about JSON here. We’re talking about object identifiers and not about methods. Within it’s intended target languages (JS/web), these object identifiers are perfectly fine and useful – e.g. for jQuery/prototype’s each() methods where they are passed as index argument.
Morhyn.8032:
Just btw. – We’re talking about JSON here. We’re talking about object identifiers and not about methods. Within it’s intended target languages (JS/web), these object identifiers are perfectly fine and useful – e.g. for jQuery/prototype’s each() methods where they are passed as index argument.
In the example that starts this thread, the method is /v1/continents.json. The result includes the method name, “continents”, as a key in the result.
JSON is merely a simple representation of the data. It does not have a target language (despite being valid JavaScript). From http://www.ietf.org/rfc/rfc4627.txt (emphasis added) —
JavaScript Object Notation (JSON) is a lightweight, text-based,
language-independent data interchange format. It was derived from
the ECMAScript Programming Language Standard. JSON defines a small
set of formatting rules for the portable representation of structured
data.
But let’s look at a concrete example using JavaScript — http://jsfiddle.net/jsumners/vH5W8/. Notice that even in JavaScript the true result of the API method call has to be dereferenced from the included key. Thus, you can’t simply loop over what is returned from the API call.
Yes, in JavaScript this isn’t so much of a problem. You can mostly just shrug it off and move on. But when you start importing this data into a language with proper types (e.g. Java in my case or C# in a previous poster’s), it adds unnecessary complications. The API could be simpler and better if it just didn’t include the method name as a key.
Heimdall.4510:
Morhyn.8032Yes, in JavaScript this isn’t so much of a problem. You can mostly just shrug it off and move on. But when you start importing this data into a language with proper types (e.g. Java in my case or C# in a previous poster’s), it adds unnecessary complications. The API could be simpler and better if it just didn’t include the method name as a key.
I really don’t get your problem with that, just parse the JSON-Object and put the result into the format you need.
I personally would also prefer raw binary data which could be simply put into a c struct but that’s not possible in all languages. I guess that’s why the GW2-Team decided to put the queryresults into a format which can be handled by all languages
Ryan.9387:
I’ve never had any issue though. I just parse it in python into a giant dictionary and if I don’t want a key I just remove it or create a new dictionary with the values I want
Killer Rhino.6794:
110% agree with the OP. The response structure is certainly kludgy, and I’m surprised that this hasn’t been more of a contentious topic here.
I suspect that most people are just used to it at this point. The good news is that there’s so few updates to the APIs that you’ll have plenty of time to work on hacks which can compensate.
OH SNAP!
Morhyn.8032:
110% agree with the OP. The response structure is certainly kludgy, and I’m surprised that this hasn’t been more of a contentious topic here.
I suspect that most people are just used to it at this point. The good news is that there’s so few updates to the APIs that you’ll have plenty of time to work on hacks which can compensate.
OH SNAP!
I was beginning to think no one else understood. Thank you for chiming in.
DarkSpirit.7046:
Let me try to answer this in the context of C#. For the first case, it is natural to deserialize to a Dictionary and for the second, a List.
In terms of lookup times, the Dictionary is usually faster than the List, unless you have only 3 items or fewer.
Yes, if you’re deserializing my suggested “fixed” serializations. With the actual API result you either:
- deserialize to a dictionary with one key who’s value is the real result
- write a custom deserializer to ignore the useless outer key
I don’t see the point in either case. Not when the API could simply return the real result very easily.
If you are deserializing it to a dictionary then you are deserializing more than just one record. Furthermore, the API goes through the web and it is therefore very slow, so unless the record changes often, you want to download/cache the records into say, a JSON file, and read them into a dictionary to increase performance.
StevenL.3761:
I find it actually quite useful to have the results collection be a named property of the root object. At least, that way you can refer to it by name.
It’s just annoying that the API is so inconsistent. For example: the “continents.json” API has this behavior whereas the “event_names.json” API doesn’t. As @Rhino suggests, you gotta write a different hack for each endpoint. It would be nice being able to reuse the same sloppy code for everything.
Kegsay.7068:
Yeah, it is pretty tedious having to go through and deal with some interesting JSON structures from the various endpoints we can hit. That being said, it’s also not difficult to just parse it out.
At this point, it would be far more damaging to change the spec and break everyone’s stuff, so I would prefer it if no changes were made. However, presenting a consistent format for all new APIs going forward would be a great idea.
smiley.1438:
At this point, it would be far more damaging to change the spec and break everyone’s stuff, so I would prefer it if no changes were made. However, presenting a consistent format for all new APIs going forward would be a great idea.
I wouldn’t mind at all if it would break stuff in favour of overall consistent responses.
These APIs are in beta, so don’t be surprised if the interfaces change, but we will try to warn you if we have to make any breaking changes.
StevenL.3761:
At this point, it would be far more damaging to change the spec and break everyone’s stuff, so I would prefer it if no changes were made. However, presenting a consistent format for all new APIs going forward would be a great idea.
The best way forward (imo) would be to go ahead and make the breaking changes, but make the improved APIs available alongside the existing APIs as v2/*.json. Then when everyone has migrated, perhaps a year later, they can take the old version offline.
Kegsay.7068:
At this point, it would be far more damaging to change the spec and break everyone’s stuff, so I would prefer it if no changes were made. However, presenting a consistent format for all new APIs going forward would be a great idea.
The best way forward (imo) would be to go ahead and make the breaking changes, but make the improved APIs available alongside the existing APIs as v2/*.json. Then when everyone has migrated, perhaps a year later, they can take the old version offline.
I like this idea.
Morhyn.8032:
At this point, it would be far more damaging to change the spec and break everyone’s stuff, so I would prefer it if no changes were made. However, presenting a consistent format for all new APIs going forward would be a great idea.
The best way forward (imo) would be to go ahead and make the breaking changes, but make the improved APIs available alongside the existing APIs as v2/*.json. Then when everyone has migrated, perhaps a year later, they can take the old version offline.
Considering the age of v1, I agree. The current version has been around long enough that it doesn’t matter if it was put out under the caveat that it is “a beta.” There are many people using it as it is, and they don’t expect drastic changes due to the long times between updates.
I didn’t make the post expecting ANet to turn around and “fix” the v1 methods. But I do hope they take it under advisement for v2.
cvpcs.5914:
It doesn’t seem too hacky to me to handle this (at least in C#). I personally use the RestSharp library to handle REST requests and the corresponding JSON deserialization. From there I just need to pass in the type that I’m deserializing to for REST to do its thing. For example, I have a basic Request class that looks like so:
using System;
using System.Collections.Generic;
using RestSharp;
namespace GuildWars2.ArenaNet.API
{
public abstract partial class Request<T>
where T : class, new()
{
public static int Timeout = 10000;
public static readonly string URL = "https://api.guildwars2.com";
public static readonly string Version = "v1";
protected abstract string APIPath { get; }
protected virtual Dictionary<string, string> APIParameters
{
get { return new Dictionary<string, string>(); }
}
protected virtual Method APIMethod
{
get { return Method.GET; }
}
public T Execute()
{
RestClient client = new RestClient();
client.BaseUrl = URL;
client.Timeout = Timeout;
RestRequest request = new RestRequest();
request.Method = APIMethod;
request.Resource = APIPath;
foreach (KeyValuePair<string, string> parameter in APIParameters)
{
request.AddParameter(parameter.Key, parameter.Value);
}
IRestResponse<T> response = client.Execute<T>(request);
if (response.StatusCode == System.Net.HttpStatusCode.OK)
return response.Data;
else
return null;
}
}
}I then have a ContinentRequest file that extends that and looks like so:
using System;
using System.Collections.Generic;
using GuildWars2.ArenaNet.Model;
namespace GuildWars2.ArenaNet.API
{
public class ContinentsRequest : TranslatableRequest<ContinentsResponse>
{
protected override string APIPath
{
get { return "/" + Version + "/continents.json"; }
}
public ContinentsRequest(LanguageCode lang = LanguageCode.EN)
: base(lang)
{ }
}
}So then RestSharp expects to be deserializing a type of ContinentResponse, which looks like so:
using System;
using System.Collections.Generic;
using GuildWars2.ArenaNet.Model;
namespace GuildWars2.ArenaNet.API
{
public class ContinentsResponse
{
public Dictionary<string, Continent> Continents { get; set; }
}
}Thus RestSharp will deserialize the response as a single item which is a dictionary of Continents:
using System;
using System.Collections.Generic;
namespace GuildWars2.ArenaNet.Model
{
public class Continent
{
public string Name { get; set; }
public List<double> ContinentDims { get; set; }
public int MinZoom { get; set; }
public int MaxZoom { get; set; }
public List<int> Floors { get; set; }
}
}So then to cycle through my list of Continents, I just need to call the following:
ContinentResponse response = new ContinentRequest().Execute();
foreach (KeyValuePair<string, Continent> kvp in response.Continents)
{
string cid = kvp.Key;
Continent c = kvp.Value;
...
}I personally like having it set up this way, as all of my requests and responses have concrete types that I can modify and add extra logic to if I wish. As you can see, the fact that they nest their response dictionary inside of the Continents variable is a non-issue. If they ever got rid of that, I could just change my ContinentsResponse class to not have any variables and simply extend the Dictionary<string, Continent> class and update any references I’m using.
You can argue about refactoring code until you’re blue in the face and there will always be other people who argue that it needs to be refactored again. Your complaint was specifically around the difficulty of deserializing the response due to the desired content being nested inside of a dictionary variable in the response object, instead of the response object simply being the dictionary variable (or a list).
It sounds more like your deserializer doesn’t handle nested objects very well, otherwise it would be a non-issue. This is very much a solved problem at this point and there should be existing libraries that can handle this quite elegantly for you in Java.
rodadams.5963:
At this point, it would be far more damaging to change the spec and break everyone’s stuff, so I would prefer it if no changes were made. However, presenting a consistent format for all new APIs going forward would be a great idea.
The best way forward (imo) would be to go ahead and make the breaking changes, but make the improved APIs available alongside the existing APIs as v2/*.json. Then when everyone has migrated, perhaps a year later, they can take the old version offline.
And while we’re at it, get rid of the whole “when we refer to one thing from another, the reference is a string, but the key is an integer”, so you just have to go str→int in some pretty annoying places.
StevenL.3761:
@cvpcs
We’re working on something quite similar over at the GW2.NET project.
I wrote the following classes over the past couple of weeks:
- ApiRequest : RestRequest
- ContinentsRequest : ApiRequest
- ApiResponse<TContent> : RestResponse<T>
- ContinentsResponse
- Continent
You seem to know your stuff. Would you be interested in becoming a contributor?
cvpcs.5914:
@StevenL
I appreciate the offer but at the moment I’m too busy to do much contributing anywhere (the code above I wrote ages ago when the APIs first came out, I haven’t really touched it since).
That said, I’ll keep it in mind if I ever decide to delve deeper into this stuff. Perhaps if ArenaNet ever gets their OAuth and related APIs published.
Lazarus.9716:
Why do you insist on including the method name in the response? As an example, if we query /v1/continents.json we know we’re getting back a list of continents. We don’t need to be told again.
That would be bad API design. Any structured information should be self-explanatory and complete so it can be included in other places (nested within another structure of information, for example).
Linking the format to your expectation of your top-level call makes the structure unnecessarily idiosyncratic and brittle. It also makes you write more code when you access the same information from other areas of the API.
Morhyn.8032:
Why do you insist on including the method name in the response? As an example, if we query /v1/continents.json we know we’re getting back a list of continents. We don’t need to be told again.
That would be bad API design. Any structured information should be self-explanatory and complete so it can be included in other places (nested within another structure of information, for example).
Linking the format to your expectation of your top-level call makes the structure unnecessarily idiosyncratic and brittle. It also makes you write more code when you access the same information from other areas of the API.
Uh, no, it would be clear API design. You have requested the list of “continents” by calling the method named “continents”. Thus, you should get back a list of, you guessed it, continents. You should not be expecting anything else, thus the inclusion of the type in the response is extraneous.
What you do with that data once you have deserialized it is up to you. You can choose to include it in another object or simply work with it directly. But you certainly don’t need the structure to be defined by the remote method.