From: Fuad Saud Date: 2013-10-15T07:44:46-03:00 Subject: [ruby-core:57871] Re: [ruby-trunk - Feature #9020] Net::HTTPResponse predicate/query methods --f46d043d673feec28a04e8c545b3 Content-Type: text/plain; charset=ISO-8859-1 Agreed. Class chechinkg is bad, duck typing is elegant and flexible. On Oct 15, 2013 7:41 AM, "timcraft (Tim Craft)" 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/ > --f46d043d673feec28a04e8c545b3 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

Agreed. Class chechinkg is bad, duck typing is elegant and flexible.

On Oct 15, 2013 7:41 AM, "timcraft (Tim Cra= ft)" <redmine@ruby-lang.or= g> wrote:

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


drbrain (Eric Hodel) wrote:
> Why use =3D=3D=3D instead of a case statement?

I agree case statements would be more common than =3D=3D=3D when testing fo= r multiple statuses. I used =3D=3D=3D in the example because it was testing= for just one status (to make it easier to compare to the other examples).<= br>
I think checking the class using case statements like in your example is fa= irly readable, but still has the issue of coupling the caller to Net::HTTP.= I think this would be an improvement:

=A0 =A0 if response.success?
=A0 =A0 =A0 response.body
=A0 =A0 elsif response.redirection?
=A0 =A0 =A0 get response['Location']
=A0 =A0 else
=A0 =A0 =A0 response.value
=A0 =A0 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:

=A0 =A0 response.ok?
=A0 =A0 response.not_found?
=A0 =A0 response.client_error?
=A0 =A0 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:

=A0 =A0 response.code.to_i =3D=3D 200

Subjectively I could say this kind of code is awkward/tedious to type, and = not very "intention revealing". More practically/objectively it&#= 39;s a potential source of error. By mistyping the "magic number"= it's possible for this expression to "silently fail" and tes= t 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, fo= r example:

=A0 =A0 Net::HTTPOK =3D=3D=3D response

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

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

=A0 =A0 response.ok?
=A0 =A0 response.not_found?

This is more concise/readable. In most cases it would be easier and "l= ess fiddly" to type out than the existing approaches presented above.<= br>
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 per= formance perspective), and it eliminates the "failing silently" i= ssue.

Compared to testing the type/class of the response object it doesn't co= uple the caller to implementation details of Net::HTTP, so it would be easi= er 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 o= f statuses, for example:

=A0 =A0 response.client_error?
=A0 =A0 response.server_error?

Similar benefits/rationale.

# IMPLEMENTATION

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

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

I can think of various different ways to implement a patch, so if this feat= ure 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.<= br>
# DISCUSSION

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



--
http://bugs.ruby-l= ang.org/
--f46d043d673feec28a04e8c545b3--