RoyaleAPI / cr-api

Clash Royale Analytics, Profiles and Insights. We no longer publish a public API. Please use the official API from Supercell.
https://RoyaleAPI.com
168 stars 20 forks source link

Create success field #45

Closed selfish closed 7 years ago

selfish commented 7 years ago

Always existing key with true value to identify good responses.

smlbiobot commented 7 years ago

When we create success field, I would like to propose that we wrap the data inside its own field, e.g.:

{
    "success": true,
    "data": {
        "tag": "C0G20PR2",
        "name": "SML",
        "trophies": 4465,
        "arena": {
            "imageURL": "/arena/league2.png",
            "arena": "League 2",
            "arenaID": 13,
            "name": "Challenger II",
            "trophyLimit": 4300
        }
    }
}

It will probably break a good amount of apps (including my own) but I think that it might be for the better since it would be in line with most best practices.

Up for discussion if you don’t agree.

isaacli430 commented 7 years ago

To be honest, there really isn't a point in having to wrap the data like that. I would prefer leaving the data alone and then just adding an endpoint.

{
    "tag": "Y99YRPYG",
    "name": "marcel_p",
    "trophies": 4944,
    "arena": {
        "imageURL": "/arena/league4.png",
        "arena": "League 4",
        "arenaID": 15,
    ....
    "success": true
}
smlbiobot commented 7 years ago

Mostly it has to do with best practices. These things are highly opinionated + subjective…

selfish commented 7 years ago

I dislike the idea of success field, and I'll explain why:

I create an API, which uses http, and every response has a response code. As we all know, code 200 means OK, which is, well, just like saying success.

I maintain this as a core part of creating a JSON API, so this is mandatory. To add a success field would mean another indication to maintain.

All that said, best practices were mentioned, and I won't object to receiving some recommended reading material to understand why everyone like this success field.

Perhaps it really helps in a different paradigm than the one I use, or in a different language/framework.

If I can understand why this is important to some, I'll make the effort of implementing it, even though, it is a pain.

But please, do explain to me, or share resources.

smlbiobot commented 7 years ago

Re: Status / success field

I guess right now it is unclear what error codes to check for. If we can assume that anything but 200 equates an error, then you I guess we can skip the status code if it is too much work.

Re: data field

I see this field used on some APIs, e.g. Tumblr, Instagram, though I would say that it is not implemented in others, e.g. Twiter, LinkedIn. I feel that it is a matter of preference. And if you think that it is unnecessary work, then we don’t have to implement it.

I talk about best practices — mostly referencing implementations by large corporations. However, I do realize that it is not universal.

For Reference:

Instagram

https://www.instagram.com/developer/endpoints/

{
    "meta": {
        "code": 200
    },
    "data": {
        ...
    },
    "pagination": {
        "next_url": "...",
        "next_max_id": "13872296"
    }
}

Tumblr

https://www.tumblr.com/docs/en/api/v2

{
   "meta": {
      "status": 200,
      "msg": "OK"
   },
   "response": { ... }
}

Twitter

https://developer.twitter.com/en/docs/tweets/timelines/api-reference/get-statuses-home_timeline.html

Doesn’t use it so it does not matter.

LinkedIn

https://developer.linkedin.com/docs/rest-api

Uses XML response so does not apply.

Google Maps API

Geolocation API

https://developers.google.com/maps/documentation/geolocation/intro

Uses detailed error code with response codes include but does not wrap data within data field.

{
 "error": {
  "errors": [
   {
    "domain": "global",
    "reason": "parseError",
    "message": "Parse Error",
   }
  ],
  "code": 400,
  "message": "Parse Error"
 }
}

Directions . API

https://developers.google.com/maps/documentation/directions/start

Gives status field but also does not wrap data within data field.

{
   "geocoded_waypoints" : [
      {
         "geocoder_status" : "OK",
         "place_id" : "ChIJRVY_etDX3IARGYLVpoq7f68",
         "types" : [
            "bus_station",
            "transit_station",
            "point_of_interest",
            "establishment"
         ]
      },
      {
         "geocoder_status" : "OK",
         "partial_match" : true,
         "place_id" : "ChIJp2Mn4E2-woARQS2FILlxUzk",
         "types" : [ "route" ]
      }
   ],
   "routes" : [
      {
         "bounds" : {
            "northeast" : {
               "lat" : 34.1330949,
               "lng" : -117.9143879
            },
            "southwest" : {
               "lat" : 33.8068768,
               "lng" : -118.3527671
            }
         },
         "copyrights" : "Map data ©2016 Google",
         "legs" : [
            {
               "distance" : {
                  "text" : "35.9 mi",
                  "value" : 57824
               },
               "duration" : {
                  "text" : "51 mins",
                  "value" : 3062
               },
               "end_address" : "Universal Studios Blvd, Los Angeles, CA 90068, USA",
               "end_location" : {
                  "lat" : 34.1330949,
                  "lng" : -118.3524442
               },
               "start_address" : "Disneyland (Harbor Blvd.), S Harbor Blvd, Anaheim, CA 92802, USA",
               "start_location" : {
                  "lat" : 33.8098177,
                  "lng" : -117.9154353
               },

  ... Additional results truncated in this example[] ...

         "overview_polyline" : {
            "points" : "knjmEnjunUbKCfEA?_@]@kMBeE@qIIoF@wH@eFFk@WOUI_@?u@j@k@`@EXLTZHh@Y`AgApAaCrCUd@cDpDuAtAoApA{YlZiBdBaIhGkFrDeCtBuFxFmIdJmOjPaChDeBlDiAdD}ApGcDxU}@hEmAxD}[tt@yNb\\yBdEqFnJqB~DeFxMgK~VsMr[uKzVoCxEsEtG}BzCkHhKWh@]t@{AxEcClLkCjLi@`CwBfHaEzJuBdEyEhIaBnCiF|K_Oz\\
            {MdZwAbDaKbUiB|CgCnDkDbEiE|FqBlDsLdXqQra@kX|m@aF|KcHtLm@pAaE~JcTxh@w\\`v@gQv`@}F`MqK`PeGzIyGfJiG~GeLhLgIpIcE~FsDrHcFfLqDzH{CxEwAbBgC|B}F|DiQzKsbBdeA{k@~\\oc@bWoKjGaEzCoEzEwDxFsUh^wJfOySx[uBnCgCbCoFlDmDvAiCr@eRzDuNxC_EvAiFpCaC|AqGpEwHzFoQnQoTrTqBlCyDnGmCfEmDpDyGzGsIzHuZzYwBpBsC`CqBlAsBbAqCxAoBrAqDdDcNfMgHbHiPtReBtCkD|GqAhBwBzBsG~FoAhAaCbDeBvD_BlEyM``@uBvKiA~DmAlCkA|B}@lBcChHoJnXcB`GoAnIS~CIjFDd]A|QMlD{@jH[vAk@`CoGxRgPzf@aBbHoB~HeMx^eDtJ}BnG{DhJU`@mBzCoCjDaAx@mAnAgCnBmAp@uAj@{Cr@wBPkB@kBSsEW{GV}BEeCWyAWwHs@qH?
            cIHkDXuDn@mCt@mE`BsH|CyAp@}AdAaAtAy@lBg@pCa@jE]fEcBhRq@pJKlCk@hLFrB@lD_@xCeA`DoBxDaHvM_FzImDzFeCpDeC|CkExDiJrHcBtAkDpDwObVuCpFeCdHoIl\\uBjIuClJsEvMyDbMqAhEoDlJ{C|J}FlZuBfLyDlXwB~QkArG_AnDiAxC{G|OgEdLaE`LkBbEwG~KgHnLoEjGgDxCaC`BuJdFkFtCgCnBuClD_HdMqEzHcBpB_C|BuEzCmPlIuE|B_EtDeBhCgAdCw@rCi@|DSfECrCAdCS~Di@jDYhA_AlC{AxCcL`U{GvM_DjFkBzBsB`BqDhBaEfAsTvEmEr@iCr@qDrAiFnCcEzCaE~D_@JmFdGQDwBvCeErEoD|BcFjC}DbEuD~D`@Zr@h@?d@Wr@}@vAgCbEaHfMqA`Cy@dAg@bAO`@gCi@w@W"
         },
         "summary" : "I-5 N and US-101 N",
         "warnings" : [],
         "waypoint_order" : []
      }
   ],
   "status" : "OK"
}
selfish commented 7 years ago

Thanks for the detailed response. Since I still don't see an actual reason to include that, let's settle for now with the following: Response code 200 is equal to success: true. Any other code means error.

If we ever identify a need for other meta info we can revise the format, but for the time being, I'd rather save the time of implementing and maintaining it, and I'm going to close the issue.