Add seek to hash for deep pluck of key

[alexbartlow]

The pattern of:

params[:user] && params[:user][:groups] && params[:user][:groups][:group_id]

... really sucks. Add a method that allows easy plucking from within a hash:

params.seek(:user, :groups, :group_id)

[steveklabnik]

I'm not sure how common this is, and I'm not sure that a recursive implementation is the best idea.

I feel like we have rejected this feature before.

[alexbartlow]

I run into this whenever I work with json data - there's always some degree of null-checking involved.

I'm fine not doing a recursive implementation for this, but I'd be curious as to why you'd feel like it's better to handle it iteratively.

[senny]

I definitely ran into this situation and built small helper function to accomplish it.

I would be interested in the thoughts of @fxn @rafaelfranca and @carlosantoniodasilva

[carlosantoniodasilva]

I've needed this before but it didn't bother me that much to the point of adding it to core, so I'm not sure it should be there. No strong opinion. Thanks.

[carlosantoniodasilva]

This needs to require try.

[alexbartlow]

I've removed the dependency on .try in the new iterative implementation.

[carlosantoniodasilva]

I made some comments anyway, it'd also need a changelog entry in case it goes in (but you can do that later if it's a yes). Thanks.

[egilburg]

I'd imagine many, if not most, Rails developers would sooner or later run into the need to extract a nested hash item. It's quite annoying to have to check for nil at each step of the way. I think it's reasonable to support this - either on the hash level, or as a special case on the singleton class of the params controller object.

I'm not sure seek is the best method name - I wouldn't have guessed it if I was facing the problem. I'd go with either deep_fetch or deep_extract (perhaps with a mutative deep_extract! to match the existing non-recursive mutative extract!)

Iterative vs recursive - iterative is usually a little bit faster and less memory intensive, so for a low-level helper function that may be run in an inner loop-scenario, it's probably better. Iterative tends to be a little bit less intuitive to read and maintain, but again, a very simple low-level helper function wouldn't be changed that often and wouldn't have many gotchas requiring understanding of its innerworkings, so most users wouldn't care about implementation any more than they care about implementation of the C-based MRI standard library.

[alexbartlow]

I changed this to an iterative implementation, added a changelog entry, and cleaned up the tests according to @carlosantoniodasilva 's comments. Thanks for the feedback Carlos.

Anyone feel like merging this?

[alexbartlow]

Oh, and @egilburg , I don't like naming it deep_fetch, since fetch allows the optional block to provide a default value. I can add another method to the pull request implementing deep_fetch with those semantics though.

[egilburg]

Needs to have API documentation similar to the other Hash and other core_ext docs.

[egilburg]

Why not improve existing implementation to allow for default block similar to how fetch does it? You can combine varargs with block, def deep_fetch(*args, &block). You'd be adding the flexibility that fetch has without losing anything, and also making your new method for recognizable for existing users by naming it similar to an existing method except for the deep prefix, which would in fact describe the only difference between the two.

The only reason I see NOT to do this is due to different variation - fetch raises if given a missing key, and I'm not sure you want this in your method.

Update - nevermind, I realized that fetch allows not only a default block but a default as 2nd argument. This will indeed cause conflicts with varargs.

[alexbartlow]

@egilburg I think it is a good idea to add an optional block to define the default value though, I've added that functionality.

[rafaelfranca]

I'm 👎 for this one. I don't see this as a common pattern, and you can already do with .try.

params.try(:[], :user).try(:[], :groups).try(:[], :groups_id)

Also, in my opinion, this is a smell.

[tenderlove]

Agree with @rafaelfranca. You can also do this with inject (if you don't want to use AS):

>> params = { :user => { :groups => { :group_id => 10 } } }
=> {:user=>{:groups=>{:group_id=>10}}}
>> [:user, :groups, :group_id].inject(params) { |hash, val| hash[val] ? hash[val] : break }
=> 10
>>

You can even do it without writing any conditionals:

>> params = { }
=> {}
>> [:user, :groups].inject(params) { |hash, val| hash.fetch(val, {}) }[:group_id]
=> nil
>> params = { :user => { :groups => { :group_id => 10 } } }
=> {:user=>{:groups=>{:group_id=>10}}}
>> [:user, :groups].inject(params) { |hash, val| hash.fetch(val, {}) }[:group_id]
=> 10
>>

[rafaelfranca]

Thank you for the contribution. ❤️

We try to not add new things to Active Support core extensions if they are not really needed.