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

Rage Against the Params #1279

Merged
merged 1 commit into from
Apr 1, 2017
Merged

Rage Against the Params #1279

merged 1 commit into from
Apr 1, 2017

Conversation

mwpastore
Copy link
Member

@mwpastore 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
Copy link

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
Copy link
Member Author

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.

@faustoct1
Copy link

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)

@faustoct1
Copy link

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
Copy link
Member Author

mwpastore commented Jun 4, 2017 via email

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

Successfully merging this pull request may close these issues.

4 participants