jira-api-restclient icon indicating copy to clipboard operation
jira-api-restclient copied to clipboard

Inconsistency in return type of API calls

Open jpastoor opened this issue 9 years ago • 3 comments

The API::api() method has a parameter called $return_as_json.

Currently I see two problems

  1. When set to true, data is not returned as JSON but as associative array instead.
  2. The API class has many helper methods that wrap around the api() method. About half of them return as associative array, the others as Result.

I haven't looked into this too deeply yet but it might be nice to

  • standardize the wrapper methods to one return type
  • and/or propogate the return_as_json by adding that option in the wrapper method parameter list as well
  • maybe rename return_as_json to return_as_array

jpastoor avatar Feb 28 '16 12:02 jpastoor

Having api method on Api class is confusing enough as it is :smile:

Looking at Api::api method the $return_as_json in fact means:

  1. get data
  2. parse it into array using json_decode
  3. return it

Without this $return_as_json parameter the same array as above is wrapped with Result class, which is some kind of iterator creation attempt, that walks over queried issues.

Maybe doing a rename on that parameter will solve that problem. We can't really introduce apiJSON method to solve it.

aik099 avatar Feb 28 '16 13:02 aik099

After pondering about this for a while I have some usable responses now:

standardize the wrapper methods to one return type

👍

The Result class obviously is tailored for representing issue list that Walker can iterate upon. For all other response types (e.g. project versions, statuses, etc.) the only Result::getResult method would return underlying array as-as.

Since it's 2.0.0 we can:

  • change default value for $return_as_json to true (from false)
  • in place (1 or 2 places most likely), where we're getting issue list back specify $return_as_json as false

and/or propogate the return_as_json by adding that option in the wrapper method parameter list as well

@jpastoor , what do you mean?

maybe rename return_as_json to return_as_array

I've created #119 with exactly that proposition.

aik099 avatar Jul 23 '16 11:07 aik099

I'll write a PR for this tomorrow

jpastoor avatar Aug 14 '16 12:08 jpastoor