Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Response class? #8

Closed
sckott opened this issue Oct 15, 2015 · 14 comments
Closed

Response class? #8

sckott opened this issue Oct 15, 2015 · 14 comments

Comments

@sckott
Copy link
Owner

sckott commented Oct 15, 2015

Make a new response class to hold results? Or stick with Faraday::Response object? Benefits of a new class is full control, but sticking with the Faraday::Response builds on all the hard work done already on that gem

@sckott sckott added this to the v0.1 milestone Oct 15, 2015
@sckott
Copy link
Owner Author

sckott commented Oct 15, 2015

@mfenner What is best practice in Ruby with respect to this question? From your point of view, would you rather get back the raw Faraday::Response object or something else? If not the Faraday::Response object, seems a new response class defined in this client would be best, with methods on the response class for parsing, response status, etc.

@mfenner
Copy link

mfenner commented Oct 15, 2015

See my utility class here: https://github.com/lagotto/lagotto/blob/master/app/models/concerns/networkable.rb. The tricky bit is error handling, as you want to catch them and give a meaningful message back to the user. I use response.body and parse the JSON (or XML) if there is no error.

@mfenner
Copy link

mfenner commented Oct 15, 2015

In other words, if there is no error you probably are only interested in response.body.

@mfenner
Copy link

mfenner commented Oct 15, 2015

@sckott
Copy link
Owner Author

sckott commented Oct 15, 2015

Thanks. Right, that is a lot of work for error handling

@mfenner
Copy link

mfenner commented Oct 15, 2015

You can probably get away with less. But I collected this over time. A simpler version is at https://github.com/crosscite/doi-metadata-search/blob/master/lib/network.rb

@sckott sckott mentioned this issue Oct 15, 2015
@sckott
Copy link
Owner Author

sckott commented Nov 5, 2015

Given cursor for paging in the API #14 I think it makes sense to have a class for the response object so that we can maintain state, esp since this cursor approach to paging is esp not very easy for a user to handle manually

@sckott
Copy link
Owner Author

sckott commented Nov 5, 2015

Would still be nice to return a hash by default if no errors since users can easily process that as needed

@mfenner
Copy link

mfenner commented Nov 5, 2015

yes, I like hash as standard response, with an :error attribute if an error occured. Thinking about JSON API, maybe the standard key for a succesful response could be :data.

@sckott
Copy link
Owner Author

sckott commented Nov 5, 2015

data is a good idea, haven't had to do that so far as no internal paing, but once that's used, will need to do that when combining results

@mfenner
Copy link

mfenner commented Nov 5, 2015

haven't thought much about it, but would be interesting to see how much you can propagate API conventions into the processing of the response.

@sckott
Copy link
Owner Author

sckott commented Nov 5, 2015

Good idea. will think about that

@sckott sckott removed this from the v0.1 milestone Nov 12, 2015
@sckott
Copy link
Owner Author

sckott commented Dec 3, 2015

will need to create a class for maintaining state for using cursor, see #14 , but can still just return hash/array

@sckott
Copy link
Owner Author

sckott commented Dec 3, 2015

closing for now, returning hash/array

@sckott sckott closed this as completed Dec 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants