Remove duplication in Ruby map with conditional entries

Question

I have a nasty (IMO) piece of code for a datatable configuration, how do i best remove the duplication here?

I've tried the obvious things I know (collapsing into one statement and trying inline conditionals) but it isn't working and I can't help thinking there must be a more elegant Ruby way here.

if show_status == "true"
 if show_requested_by == "true"
 records.map do |record|
 {
 warning: as_warning(record),
 updated_at: as_time_ago(record.updated_at),
 status: as_status(record.status),
 user: record.user.nil? ? "" : record.user.formal_name,
 manager: record.manager.nil? ? "" : record.manager.formal_name,
 first_date: as_date(record.first_date),
 last_date: as_date(record.last_date),
 days: as_number(record.total_days),
 hours: as_number(record.total_hours),
 units: as_number(record.total_units),
 value: as_number(record.total_value),
 comment: as_comment(record.comment),
 actions: as_actions(record)
 }
 end
 else
 records.map do |record|
 {
 warning: as_warning(record),
 updated_at: as_time_ago(record.updated_at),
 status: as_status(record.status),
 manager: record.manager.nil? ? "" : record.manager.formal_name,
 first_date: as_date(record.first_date),
 last_date: as_date(record.last_date),
 days: as_number(record.total_days),
 hours: as_number(record.total_hours),
 units: as_number(record.total_units),
 value: as_number(record.total_value),
 comment: as_comment(record.comment),
 actions: as_actions(record)
 }
 end
 end
else
 if show_requested_by == "true"
 records.map do |record|
 {
 warning: as_warning(record),
 updated_at: as_time_ago(record.updated_at),
 user: record.user.nil? ? "" : record.user.formal_name,
 manager: record.manager.nil? ? "" : record.manager.formal_name,
 first_date: as_date(record.first_date),
 last_date: as_date(record.last_date),
 days: as_number(record.total_days),
 hours: as_number(record.total_hours),
 units: as_number(record.total_units),
 value: as_number(record.total_value),
 comment: as_comment(record.comment),
 actions: as_actions(record)
 }
 end
 else
 records.map do |record|
 {
 warning: as_warning(record),
 updated_at: as_time_ago(record.updated_at),
 manager: record.manager.nil? ? "" : record.manager.formal_name,
 first_date: as_date(record.first_date),
 last_date: as_date(record.last_date),
 days: as_number(record.total_days),
 hours: as_number(record.total_hours),
 units: as_number(record.total_units),
 value: as_number(record.total_value),
 comment: as_comment(record.comment),
 actions: as_actions(record)
 }
 end
 end
end

Answer 1

Another way to do this would be to inline the conditionals, returning nil when they are not met and use compact or compact! to remove those entries at the end. Something like:

records.map do |record|
 {
 warning: as_warning(record),
 updated_at: as_time_ago(record.updated_at),
 status: show_status == 'true' ? as_status(record.status) : nil,
 user: show_requested_by=='true' ? (record.user.nil? ? "" : record.user.formal_name) : nil,
 manager: record.manager.nil? ? "" : record.manager.formal_name,
 first_date: as_date(record.first_date),
 last_date: as_date(record.last_date),
 days: as_number(record.total_days),
 hours: as_number(record.total_hours),
 units: as_number(record.total_units),
 value: as_number(record.total_value),
 comment: as_comment(record.comment),
 actions: as_actions(record)
 }.compact
}

Note: If any of your entries can be nil (and it is important to keep them in the hash) then this won't work.

Update: As an alternative that handles nils that are expected you could also go for something like:

records.map do |record|
 {
 status: show_status == 'true' ? as_status(record.status) : nil,
 user: show_requested_by=='true' ? (record.user.nil? ? "" : record.user.formal_name) : nil,
 }.compact
 .merge {
 warning: as_warning(record),
 updated_at: as_time_ago(record.updated_at),
 manager: record.manager.nil? ? "" : record.manager.formal_name,
 first_date: as_date(record.first_date),
 last_date: as_date(record.last_date),
 days: as_number(record.total_days),
 hours: as_number(record.total_hours),
 units: as_number(record.total_units),
 value: as_number(record.total_value),
 comment: as_comment(record.comment),
 actions: as_actions(record)
 }
}

Although, if the user or status, should be nil they still won't be handled correctly.

• Craig McGuff: yes, this works nicely, as you say just needed to ensure necessary nils become blanks instead.
• thesecretmaster: Setting those two values to nil will make no difference because attempting to access them if they do not exist will still return nil. It only matters if you're either 1) Checking against the array of keys or 2) Changing the hash defaults.
• Marc Rohloff: There are lots of functions that chek for nils, for example in ActiveRecord, User.assign({login: nil}) is not the same as User.assign({})

Answer 2

This is just a rough thought, so it's untested, but here goes:

A vast majority of the hash is the same every time. So, that vast majority can be stuck into a method. There are two fields that are conditionally added to that hash based on their values ('true' or... not). So, we can create a method which takes a record as an argument and the two 'true'/not options and generates a hash accordingly:

def record_for(record, status: false, requested_by: false)
 record_hash = {
 warning: as_warning(record),
 updated_at: as_time_ago(record.updated_at),
 manager: record.manager.nil? ? "" : record.manager.formal_name,
 first_date: as_date(record.first_date),
 last_date: as_date(record.last_date),
 days: as_number(record.total_days),
 hours: as_number(record.total_hours),
 units: as_number(record.total_units),
 value: as_number(record.total_value),
 comment: as_comment(record.comment),
 actions: as_actions(record)
 }
 record_hash[:status] = as_status(record.status) if status
 record_hash[:user] = record.user.nil? ? "" : record.user.formal_name if requested_by
 record_hash
end

Then, all that remains is to convert the strings to booleans:

def to_bool(str)
 return true if str == 'true'
 false
end

and then iterate over the records:

status = to_bool(show_status)
requested_by = to_bool(show_requested_by)
records.map { |r| record_for(r, status: status, requested_by: requested_by) }

I'd be happy to go more in depth and test the code if you provided a set of test records that I could use or some larger context this code would run in.