If we cache params into a local var in an action, will it help or its the same?

408 Views Asked by At

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!

4

There are 4 best solutions below

0
On BEST ANSWER

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).

0
On

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.

1
On

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)

0
On

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. :-)