Feature #9020: Net::HTTPResponse predicate/query methods

• Status: Assigned
• Assignee: naruse (Yui NARUSE)
• Target version: -

[timcraft (Tim Craft)]

SUMMARY

I would like to propose adding predicate/query methods to Net::HTTPResponse for testing the status/type of response. For example:

response.ok?
response.not_found?
response.client_error?
response.server_error?

BACKGROUND

The approach I've most commonly used/encountered for testing the status of a response is to compare with an integer, for example:

response.code.to_i == 200

Subjectively I could say this kind of code is awkward/tedious to type, and not very "intention revealing". More practically/objectively it's a potential source of error. By mistyping the "magic number" it's possible for this expression to "silently fail" and test the wrong status. That would be an easy thing to spot in these examples, but much more difficult to track down within a typical codebase.

Another approach would be to test the type/class of the response object, for example:

Net::HTTPOK === response

Subjectively I would say this doesn't feel very Ruby-ish. More practically/objectively it tightly couples the caller to the implementation details of Net::HTTP, making it difficult to stub or swap in a different library.

PROPOSAL

I would like to propose adding predicate/query methods to Net::HTTPResponse in order to encapsulate the implementation details of testing for different statuses and to provide a more abstract interface to the caller. For example:

response.ok?
response.not_found?

This is more concise/readable. In most cases it would be easier and "less fiddly" to type out than the existing approaches presented above.

Compared to testing with integers it is one method call instead of three (I'm considering that as better from a readability perspective, not a performance perspective), and it eliminates the "failing silently" issue.

Compared to testing the type/class of the response object it doesn't couple the caller to implementation details of Net::HTTP, so it would be easier to stub or swap-in another library that provides the same interface.

Overall it feels much simpler and much more Ruby-ish.

In addition I would propose adding a few extra methods to test for ranges of statuses, for example:

response.client_error?
response.server_error?

Similar benefits/rationale.

IMPLEMENTATION

I have already been using methods like this in some gems, and I have created a "proof of concept" implementation which monkey-patches Net::HTTP to test the idea out. Available here:

http://rubygems.org/gems/net-http-predicates
https://github.com/timcraft/net-http-predicates

I can think of various different ways to implement a patch, so if this feature would be accepted into ruby-trunk I would welcome suggestions/guidance on a preferred implementation.

These changes would be backwards compatible and straightforward to provide as a backport, either in the backports library/gem or as a standalone gem.

DISCUSSION

Before discussing how to implement this patch I would like to get people's thoughts on the idea/proposal, and some indication of whether this could be accepted into ruby-trunk (or not). If it would be accepted I'm happy to write/submit the patch itself.

[drbrain (Eric Hodel)]

• Description updated (diff)
• Status changed from Open to Assigned
• Assignee set to naruse (Yui NARUSE)
• Target version set to 2.6

=begin
Why use === instead of a case statement?

case response
when Net::HTTPSuccess then
response.body
when Net::HTTPRedirect then
get response['Location']
else
response.value # raises exception
end
=end

[timcraft (Tim Craft)]

drbrain (Eric Hodel) wrote:

Why use === instead of a case statement?

I agree case statements would be more common than === when testing for multiple statuses. I used === in the example because it was testing for just one status (to make it easier to compare to the other examples).

I think checking the class using case statements like in your example is fairly readable, but still has the issue of coupling the caller to Net::HTTP. I think this would be an improvement:

if response.success?
  response.body
elsif response.redirection?
  get response['Location']
else
  response.value
end

[fuadksd (Fuad Saud)]

Agreed. Class chechinkg is bad, duck typing is elegant and flexible.
On Oct 15, 2013 7:41 AM, "timcraft (Tim Craft)" [e-mail]
wrote:

Issue #9020 has been updated by timcraft (Tim Craft).

drbrain (Eric Hodel) wrote:

Why use === instead of a case statement?

I agree case statements would be more common than === when testing for
multiple statuses. I used === in the example because it was testing for
just one status (to make it easier to compare to the other examples).

I think checking the class using case statements like in your example is
fairly readable, but still has the issue of coupling the caller to
Net::HTTP. I think this would be an improvement:

if response.success?
  response.body
elsif response.redirection?
  get response['Location']
else
  response.value
end

---

Feature #9020: Net::HTTPResponse predicate/query methods
https://bugs.ruby-lang.org/issues/9020#change-42466

Author: timcraft (Tim Craft)
Status: Assigned
Priority: Normal
Assignee: naruse (Yui NARUSE)
Category: lib
Target version: next minor

SUMMARY

I would like to propose adding predicate/query methods to
Net::HTTPResponse for testing the status/type of response. For example:

response.ok?
response.not_found?
response.client_error?
response.server_error?

BACKGROUND

The approach I've most commonly used/encountered for testing the status of
a response is to compare with an integer, for example:

response.code.to_i == 200

Subjectively I could say this kind of code is awkward/tedious to type, and
not very "intention revealing". More practically/objectively it's a
potential source of error. By mistyping the "magic number" it's possible
for this expression to "silently fail" and test the wrong status. That
would be an easy thing to spot in these examples, but much more difficult
to track down within a typical codebase.

Another approach would be to test the type/class of the response object,
for example:

Net::HTTPOK === response

Subjectively I would say this doesn't feel very Ruby-ish. More
practically/objectively it tightly couples the caller to the implementation
details of Net::HTTP, making it difficult to stub or swap in a different
library.

PROPOSAL

I would like to propose adding predicate/query methods to
Net::HTTPResponse in order to encapsulate the implementation details of
testing for different statuses and to provide a more abstract interface to
the caller. For example:

response.ok?
response.not_found?

This is more concise/readable. In most cases it would be easier and "less
fiddly" to type out than the existing approaches presented above.

Compared to testing with integers it is one method call instead of three
(I'm considering that as better from a readability perspective, not a
performance perspective), and it eliminates the "failing silently" issue.

Compared to testing the type/class of the response object it doesn't
couple the caller to implementation details of Net::HTTP, so it would be
easier to stub or swap-in another library that provides the same interface.

Overall it feels much simpler and much more Ruby-ish.

In addition I would propose adding a few extra methods to test for ranges
of statuses, for example:

response.client_error?
response.server_error?

Similar benefits/rationale.

IMPLEMENTATION

I have already been using methods like this in some gems, and I have
created a "proof of concept" implementation which monkey-patches Net::HTTP
to test the idea out. Available here:

http://rubygems.org/gems/net-http-predicates
https://github.com/timcraft/net-http-predicates

I can think of various different ways to implement a patch, so if this
feature would be accepted into ruby-trunk I would welcome
suggestions/guidance on a preferred implementation.

These changes would be backwards compatible and straightforward to provide
as a backport, either in the backports library/gem or as a standalone gem.

DISCUSSION

Before discussing how to implement this patch I would like to get people's
thoughts on the idea/proposal, and some indication of whether this could be
accepted into ruby-trunk (or not). If it would be accepted I'm happy to
write/submit the patch itself.

--
http://bugs.ruby-lang.org/

[drbrain (Eric Hodel)]

• Description updated (diff)

What libraries implement the Net::HTTP API that require duck-typing?

Do you have a patch for this feature request?

PS: if you respond via email, trim our the context. We don't need repeated copies of the history in the issue tracker.

[timcraft (Tim Craft)]

I'm not aware of any libraries that implement this interface currently. If Net::HTTP implemented these methods and other libraries implemented them, then it would be possible to switch between libraries without changing those particular lines of code (obviously some other lines of code would still need to be changed). A hypothetical/potential benefit at this point.

I haven't written a patch yet, and "adding significant number of lines of code to Net::HTTP source" could be considered as an argument against, but I think this is where this functionality belongs. I can think of various ways to define/implement the methods, so would welcome any feedback on that (I've not written a Ruby patch before), but there doesn't seem to be much point in having that discussion unless there is support for adding the feature.

[timcraft (Tim Craft)]

• File patch.diff patch.diff added

Adding proposed patch, which takes advantage of recent frozen string optimization.

[naruse (Yui NARUSE)]

• Target version deleted (2.6)