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

Rage Against the Params #1279

Merged
merged 1 commit into from Apr 1, 2017

Conversation

Projects
None yet
4 participants
@mwpastore
Member

mwpastore commented Mar 31, 2017

Have you ever been frustrated that params.has_key?(:sym) works, but not params.key?(:sym)?

Have you ever tried to use params.dig(..) with symbol keys? How about #fetch or #values_at?

Does this snippet of code make your blood pressure spike?

params['foo'] = "hello"
params[:foo] # => "hello"
params['foo'] # => "hello"
params[:bar] = "world"
params[:bar] # => "world"
params['bar'] # => nil # oops!

How about the fact that query parameters are mapped to string keys, but Sinatra's internal captures parameter is mapped to a symbol key? So even if you commit to using all string keys for params, you may still run into issues.

Our IndifferentHash implementation is simultaneously too indifferent and not indifferent enough. One might say it is indifferently indifferent! I propose we make it fully-indifferent, and remove as much "astonishment" as humanly possible. Here is a PR that does just that.

@b264

This comment has been minimized.

Show comment
Hide comment
@b264

b264 Mar 31, 2017

This rather blurs the line between the fundamental difference between symbols and strings, huh? The whole point of symbols is that you could replace every symbol, say :foo with another one, say :bar and your code would function identically. So def foo would also become def bar and any calls to it as well. Think of a symbol as "identifier number 1" where its specific value is irrelevant, where as a string its specific value is the thing that matters, say for example in an http request to another application. All of this indifferentness introduced to the Ruby community is kind-of eliminating the need for symbols at all in the Ruby language. Not saying it's good or bad, just pointing it out. Their entire purpose for existing has now become moot. (also frozen string literals have reinforced their coming obsolescence)

b264 commented Mar 31, 2017

This rather blurs the line between the fundamental difference between symbols and strings, huh? The whole point of symbols is that you could replace every symbol, say :foo with another one, say :bar and your code would function identically. So def foo would also become def bar and any calls to it as well. Think of a symbol as "identifier number 1" where its specific value is irrelevant, where as a string its specific value is the thing that matters, say for example in an http request to another application. All of this indifferentness introduced to the Ruby community is kind-of eliminating the need for symbols at all in the Ruby language. Not saying it's good or bad, just pointing it out. Their entire purpose for existing has now become moot. (also frozen string literals have reinforced their coming obsolescence)

@mwpastore

This comment has been minimized.

Show comment
Hide comment
@mwpastore

mwpastore Mar 31, 2017

Member

@b264 String hash keys are automatically frozen, and have been for quite some time:

$ ruby --version
ruby 1.9.3p551 (2014-11-13 revision 48407) [x86_64-darwin16.5.0]
$ ruby -e 'puts Hash[?a=>1].keys.first.frozen?'
true

So in practice, string and symbol hash keys are functionally equivalent, and I believe they perform just as well as each other.

This particular "problem" that Sinatra tries to solve with IndifferentHash (and Rails solves with HashWithIndifferentAccess) is due to Rack always using string keys to store the parsed query parameter string. Most Rubyists prefer to use symbol keys in their code, so this pattern of casting symbols to strings has developed over time. In a perfect world, Rack would (optionally?) use symbol keys for this data structure, but I guess that would break too many things.

I'd say it's fairly limited to a handful of use cases—mostly this one—so I don't see it as endemic, but that's just my perception. Most Rubyists I've observed seem to grok symbols and prefer to use them wherever possible.

Member

mwpastore commented Mar 31, 2017

@b264 String hash keys are automatically frozen, and have been for quite some time:

$ ruby --version
ruby 1.9.3p551 (2014-11-13 revision 48407) [x86_64-darwin16.5.0]
$ ruby -e 'puts Hash[?a=>1].keys.first.frozen?'
true

So in practice, string and symbol hash keys are functionally equivalent, and I believe they perform just as well as each other.

This particular "problem" that Sinatra tries to solve with IndifferentHash (and Rails solves with HashWithIndifferentAccess) is due to Rack always using string keys to store the parsed query parameter string. Most Rubyists prefer to use symbol keys in their code, so this pattern of casting symbols to strings has developed over time. In a perfect world, Rack would (optionally?) use symbol keys for this data structure, but I guess that would break too many things.

I'd say it's fairly limited to a handful of use cases—mostly this one—so I don't see it as endemic, but that's just my perception. Most Rubyists I've observed seem to grok symbols and prefer to use them wherever possible.

@zzak zzak merged commit 4194577 into sinatra:master Apr 1, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@faustoct

This comment has been minimized.

Show comment
Hide comment
@faustoct

faustoct May 22, 2017

it's fixed on 2.0.0.rc2 but not working at all on 2.0.0.rc6.
also report the issue here padrino/padrino-framework#2132 (comment)

faustoct commented May 22, 2017

it's fixed on 2.0.0.rc2 but not working at all on 2.0.0.rc6.
also report the issue here padrino/padrino-framework#2132 (comment)

@faustoct

This comment has been minimized.

Show comment
Hide comment
@faustoct

faustoct commented Jun 4, 2017

hash problems accessing hash in post request through params as described
here https://stackoverflow.com/questions/14540729/how-do-i-map-an-application-json-post-request-to-the-params-in-rails

@mwpastore

This comment has been minimized.

Show comment
Hide comment
@mwpastore

mwpastore Jun 4, 2017

Member
Member

mwpastore commented Jun 4, 2017

@mwpastore mwpastore deleted the mwpastore:full-indifference branch Jun 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment