MMP
MMP

Reputation: 556

Attempting to replace ruby if statement with ternary, code breaks

I have an array user_list that contains users. Each user is a hash and has a symbol key :name.

I am working with a spreadsheet, and for a row, the first column aka row[0] contains a name. If I want to update a user's hash with data from the remaining columns of a row, I need to retrieve the correct user hash from user_list.

Here below is code that works:

def self.retrieve_user(user_list, row)
    user_list.each do |user|
      if user[:name] == row[0]
        return user
      end
    end
end

I was attempting to replace it with this:

def self.retrieve_user(user_list, row)
    user_list.each do |user|
      user[:name] == row[0] ? user : nil
    end
end

And now my program breaks. What am I missing regarding the ternary operator?

Edit: I am trying to do this because of https://github.com/bbatsov/ruby-style-guide

Upvotes: 0

Views: 101

Answers (2)

Nimir
Nimir

Reputation: 5839

I would suggest using Enumerable#find

From the docs:

Passes each entry in enum to block. Returns the first for which block is not false. If no object matches, calls ifnone and returns its result when it is specified, or returns nil otherwise.

Here is how it looks:

def self.retrieve_user(user_list, row)
  user_list.find{|user| user[:name] == row[0] }
end

Upvotes: 2

Suhail Patel
Suhail Patel

Reputation: 13694

In the first version of your program, you were iterating through your user list and when you found the user you wanted, you returned the user to whoever called the function (thus immediately breaking out the each loop and also the retrieve_user function).

In your second version, you aren't explicitly returning any values, nothing is being done to that user object you access, it just continues to execute the loop till it gets to the end of the user_list and then it returns theuser_list` implicitly (this is a Ruby feature). See the following blog post which explains it well: http://www.thedwick.com/2012/08/ruby-implicit-returns-do-as-the-romans-do/

You could replace the code with the following to achieve what you initially wanted in one line

def self.retrieve_user(user_list, row)
    user_list.each do |user|
       return user if user[:name] == row[0]
    end
end

You should also consider the case where none of the users in the user list match the name of the user (unless you are absolutely sure this can't happen). In the above case, the entire user list will be implicitly returned as the return user will not execute since the condition is always false. You could add an explicit return nil at the end if nothing should be returned:

def self.retrieve_user(user_list, row)
    user_list.each do |user|
       return user if user[:name] == row[0]
    end
    return nil
end

Upvotes: 3

Related Questions