Map missing arrays from nil to [] ?

[eiked]

release = discogs.get_release(...)

sometimes includes release.identifiers but sometimes does not.
(aka release.identifiers == nil)

I'd like to suggest to add some postprocessing to the Discogs::Wrapper
aka: release.identifiers=nil should automagically be mapped to release.identifiers={}

And let's look up all other pieces of the responses where such things might happen.

One might argue, that nil is the proper value here and we should not mess with this.

But on the other hand, the wrapper is ment to make life easy,
and it would be much more convenient, DRY and fool proof.

So actually the contract for release.identifiers would change from:
"might also return nil" to "always returns a hash"
which lessens the clients of the api to check for nil.

[buntine]

That is how the original version of this Gem worked. There are lots of undocumented fields, though.

We could probably come up with something that builds a hashie object and then mixes in defaults depending on the type of Discogs resource we are building.

data = get_release
release = Hashie.build(data)
release = mixin_defaults(release, :release)

[eiked]

Hi Andrew,

I’ve seen the old version,
you have improved it a lot.

I’m heading for making it fool proof and fully reliable.

I saw this:
raise_unknown_resource(path) if response.code == "404"
raise_authentication_error(path) if response.code == "401"
raise_internal_server_error if response.code == "500"

But this does not yet catch all possible failures

We should come up with a definition,
as how that api should behave.

Actually throwing exceptions is a good thing if something really goes wrong,
But then in ruby, we rarely use exceptions at all (it’s so much java fuck)

I actually believe, it should be an optinon when creating the stub object.
aka: Discogs.new(exceptions:true)

While otherwise the stub should not throw exceptions at all,
but only respond with nil when there was some problem.

But then there should be some error object, saved in the stub object,
which should reflect the last error from the last request.

These are obviously two very different ways of handling errors.
But we can easily implement both of them.

---

I have to get my ass up, to push.

~eike

Am 20.06.2014 um 01:12 schrieb Andrew Buntine notifications@github.com:

That is how the original version of this Gem worked. There are lots of undocumented fields, though.

We could probably come up with something that builds a hashie object and then mixes in defaults depending on the type of Discogs resource we are building.

data = get_release
release = Hashie.build(data)
release = mixin_defaults(release, :release)

—
Reply to this email directly or view it on GitHub.

[eiked]

Hi Andrew,

I forked on
https://github.com/eiked/discogs
to apply my changes.

I’ll first try to fix my reported bugs as unintrusive as can be.

I imagine to take a second step then, splitting out Discogs::Connection, Discogs::Request and Discogs::Response from the Discogs::Wrapper, but leaving the Wrapper api intact.

Actually the discogs way of handling errors is less than robust today ...
(see http://www.discogs.com/forum/thread/53ada4b7d07b091fe8d4f48a)

I could imagine to improve on that on the client side by prechecking the request data with something like json-schema…

I write on the discogs api forum (as rockers)
http://www.discogs.com/forum/topic/1082

Am 20.06.2014 um 01:12 schrieb Andrew Buntine notifications@github.com:

That is how the original version of this Gem worked. There are lots of undocumented fields, though.

We could probably come up with something that builds a hashie object and then mixes in defaults depending on the type of Discogs resource we are building.

data = get_release
release = Hashie.build(data)
release = mixin_defaults(release, :release)

—
Reply to this email directly or view it on GitHub.