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

Marc Rohloff · Accepted Answer · 2018-01-04 16:18:49Z

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.

yes, this works nicely, as you say just needed to ensure necessary nils become blanks instead. — Craig McGuff, Jan 4 '18 at 7:06
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. — thesecretmaster, Jan 6 '18 at 14:41
There are lots of functions that chek for nils, for example in ActiveRecord, User.assign({login: nil}) is not the same as User.assign({}) — Marc Rohloff, Jan 7 '18 at 0:38

thesecretmaster · Accepted Answer · 2018-01-03 23:16:32Z

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.

Stack Exchange Network

current community

your communities

more stack exchange communities

Remove duplication in Ruby map with conditional entries

2 Answers 2

Your Answer

Not the answer you're looking for? Browse other questions tagged ruby array or ask your own question.

Hot Network Questions

Remove duplication in Ruby map with conditional entries

2 Answers 2

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged ruby array or ask your own question.

Related

Hot Network Questions