views:

95

answers:

4

So we run a code quality tool called reek once in a while as part of our project. The tool basically looks for code smells and reports them. Here, we observed that we get "Duplication" smell every time we try to access a key in params more than once (As if we are making a method-call twice with same parameters or we are duplicating an if condition etc). However, params is just a Hash, right? Other hashes don't get duplication smell when their keys are accessed more than once.

Why is this so? What are params exactly? Does it make sense to cache params in a local variable then use them? Will it help or its the same? Or is there something wrong with the tool? Help!

A: 

params[:foo] is a method-call to Hash#[], so reek is correct. I'm not familiar with reek, so I can't tell why other Hash accesses don't get counted the same. Hash#[] should be fast enough that you don't need to store it in a local variable unless you're in a very performance critical part of your code.

The only difference between the params Hash and a regular Hash is that it uses with_indifferent_access, meaning you can access any key with a String or a Symbol.

mutle
+1  A: 

params is a method call that does a @params ||= @request.params

It might be that it thinks params is a complicated method, so it wants you to try and cache it in a variable, but, dont think that would be worth it especially since it is memoized (based on my rack_process.rb from Rails 2.2)

Omar Qureshi
Thanks Omar. As Kevin pointed out, params being a kind of a DTO, its different than any other object and Reek doesn't know that yet.
Chirantan
+1  A: 

With the current version it's best to run Reek only on your app/models folder, because it raises false positives against views and controllers.

params is a kind of DTO (data transfer object) close to the system boundary, and so its characteristics should be different than regular code. But Reek doesn't know that (yet). I plan to improve Reek in the near future so that it plays better with Rails. For now though, your best bet is to restrict it to looking at app/models (and maybe app/helpers and lib).

kevinrutherford
A: 

I believe every time you call params, there is an initialization step which generates method calls, i suppose you can try creating a params and checking number of calls. this could be blind guess. :-)