2

Lets say I have user input that can be either this:

input = { user_id: 5, ... }

or this:

input = { app_id: 5, ... }

And I want to return either :user_id or :app_id depending on which is provided. I can do this:

(input.keys & [:user_id, :app_id]).first

Is there a more elegant, more rubyish, idiomatic way of doing this?

Is this better or worse than above?:

input.slice(:user_id, :app_id).keys.first

(Answers don't need to be strictly from Ruby 2.2 stdlib, Rails methods welcome as well)

Ben
  • 432
  • 5
  • 16
  • Yours is elegant enough, valud use case for set intersection. Or do you want a **value** from the hash, not the key? – D-side Dec 04 '15 at 20:22
  • No, I want the key, not the value. Thanks. – Ben Dec 04 '15 at 20:25
  • I might do `val = input[:user_id] || input[:app_id]` personally – rainkinz Dec 04 '15 at 20:30
  • @rainkinz that's precisely I was asking about, and no, this is not a solution. – D-side Dec 04 '15 at 20:30
  • @D-side oops sorry reread the question. I see what you're saying, Definitely not a solution to the question – rainkinz Dec 04 '15 at 20:31
  • I would add that a more elegant solution should also be able to scale elegantly too, so if there's now a foo_id, it should be easy to extend. – Ben Dec 04 '15 at 20:33
  • 3
    FWIW `Hash#slice` is Rails' ActiveSupport, not Ruby standard library. – D-side Dec 04 '15 at 20:34
  • @D-side, true, will edit the question to include Rails methods and add Rails tag to the question. – Ben Dec 04 '15 at 20:36

3 Answers3

5

I would solve it the other way round using find and has_key?:

[:user_id, :app_id].find { |k| input.has_key?(k) }
Stefan
  • 109,145
  • 14
  • 143
  • 218
  • 1
    Personally I think that's less elegant, not more elegant. Whatever "elegant" means :) This seems more explicit, yes, but less terse. – Ben Dec 04 '15 at 20:34
  • 1
    @Ben at least it does less allocations, so it should be more performant. – D-side Dec 04 '15 at 20:36
  • @D-side, ah yes, good point, there's an extra array created by the intersection. – Ben Dec 04 '15 at 20:43
  • I expect Stefan did it this way because it's efficient and reads well, not because it's elegant. As to whether it is "performant" I can't say, for there is no such word in my dictionary. It's efficient because `find` stops looking when it finds a match. `Hash#has_key?` aka `key?` and `include?`. – Cary Swoveland Dec 05 '15 at 14:52
  • It's very efficient: `input.has_key?(k)` is called at most two times. [The OP's code](http://stackoverflow.com/q/34096753/477037) creates an intermediate array containing all the keys from `input` in order to perform a set intersection, which creates a temporary hash to determine the common elements. [Israveri's code](http://stackoverflow.com/a/34097336/477037) (potentially) traverses every key / value pair in `input` and calls `KEYS.include?(k)` for each key in `input`. – Stefan Dec 05 '15 at 15:21
1

Your solution is good enough. An alternative that I would prefer:

input = { app_id: 5, ... }

KEYS = [:app_id, :user_id, :foo_id]

input.find { |key, value| KEYS.include? key }

In that way, you keep the what you want separated from the how you want it. You could even make KEYS be assigned from a file so that you wouldn't even have to open the code to add or remove keys from the lookup. But that may be overengineering.

I would look for a better name for KEYS tough. Naming is hard.

  • Yes, thanks for the suggestion. This is just a simple example to isolate the code I'm trying to improve. The real world code is more complex and does have the valid keys defined in a Hash constant. What I'm actually building is an adapter on top of CanCan and Pundit so I can use either through a single interface. – Ben Dec 04 '15 at 21:07
  • 1
    I wouldn't prefer this solution because existence lookups on hashes are typically faster. But the current two answers are an example of one way being applied in different directions. Worth looking at both. – D-side Dec 04 '15 at 23:00
1

Why not just:

input.key?(:user_id) ? :user_id : :app_id

? Am I missing something?

Cary Swoveland
  • 106,649
  • 6
  • 63
  • 100
  • @Ben, I don't understand your point. You obviously could wrap it in a method whose arguments are the two keys. That's the case for all the answers. – Cary Swoveland Dec 05 '15 at 08:32
  • I mean that if we need to extend this code to look for additional keys, the ternary structure no longer works, it is no longer two possible responses, whereas some of the other answers here will scale by just adding another element to the array. .key?() takes a single argument, which cannot be an array, so I don't see any alternative when extending your solution other than to add more else clauses. – Ben Dec 05 '15 at 12:00
  • 1
    Your question is quite specific that one of two given keys is present. If it is one of two or more keys, you need to say so. It's like you asking if you should wear the red hat or blue hat, me responding "the red one" and you saying "but what about the green hat?". – Cary Swoveland Dec 05 '15 at 14:31