Acquiring a hash containing a key and a value

Question

I am new to Ruby and not familiar with a lot of Array features. I feel like this code can be written in a couple of fewer lines.

The idea is: I have a Definition class and a Field class. I abstracted them as Hashes to make my life easier although they should be classes.

• Definition contains an Id and a Name
• Field contains an Id and a Value

I want a resulting Hash that contains a Name key for each corresponding Value. However, if the name is repeated, then I need to include the Id in the Name.

Here's the code for generating mock objects:

# name and id
def mock_definitions
  d = []

  d << {
    :name => "custom def 1",
    :id   => 1
  }

  d << {
    :name => "custom def 2",
    :id   => 2
  }

  d << {
    :name => "custom def 2",
    :id   => 3
  }

  d
end

def mock_fields
  f = []

  f << {
    :id => 1,
    :value => "Value For Custom Definition 1"
  }

  f << {
    :id => 2,
    :value => "Value For Custom Definition 2"
  }

  f << {
    :id => 3,
    :value => "Value For Custom Definition 3"
  }

  f
end

def all_mocks
  return mock_definitions, mock_fields
end
############# END OF MOCK ###############

and the main logic preceded by helper functions:

def build_counter_cache(definitions = [])
  counter_cache = Hash.new

  definitions.each do |d|
    if counter_cache.has_key?(d[:name])
      counter_cache[d[:name]] +=1 
    else
      counter_cache[d[:name]] = 1
    end
  end 

  counter_cache
end

def build_fields_cache(fields = [])
  fields_cache = Hash.new

  fields.each do |f|
    fields_cache[f[:id]] = f[:value]
  end

  fields_cache
end

def build_search_to_replace_mapping(fields = [], definitions = [])
  counter_cache = build_counter_cache(definitions)
  custom_fields_cache = build_fields_cache(fields)

  mapping = Hash.new

  definitions.each do |d|
    search_text = ''
    replace_text = ''

    if counter_cache[d[:name]] == 1
      search_text = "{{#{d[:name]}}}"
    else
      search_text = "{{#{d[:name]}_#{d[:id]}}}"
    end

    if custom_fields_cache.has_key?(d[:id])
      replace_text = custom_fields_cache[d[:id]]
    end

    #puts "#{search_text} maps to #{replace_text}"
    mapping[search_text] = replace_text
  end

  mapping
end

### main logic begins here ###
# mocks
definitions, fields = all_mocks
mapping = build_search_to_replace_mapping(fields, definitions)

mapping.each do |key, value|
  puts "#{key} *** maps to *** #{value}"
end

Answer 1

This is just about your current code; existing answers have good suggestions for ways to do the entire thing differently.

As sschmeck notes, you can make good use of each_with_object here, even without radically changing your code. For instance, your helpers could be:

def build_counter_cache(definitions = [])
  counter_cache = Hash.new { 0 }
  definitions.each_with_object(counter_cache) do |d, cache|
    cache[d[:name]] += 1
  end
end

def build_fields_cache(fields = [])
  fields.each_with_object({}) do |f, cache|
    cache[f[:id]] = f[:value]
  end
end

The Hash.new { 0 } trick creates a hash that will return 0 for unknown/unset keys. So you don't need the if else in your block. (Note that the hash-with-fallback-value can bite you; it always returns the same object reference. In this case, it works, since we're using an immutable integer, but if you were to say Hash.new { [] }, you'd be getting the same array each time.)

You also have a pretty redundant method in the form of all_mocks. Really the same thing can be achieved with:

definitions, fields = mock_definitions, mock_fields

or, more readably, simply say:

definitions = mock_definitions
fields = mock_fields

All the all_mocks method does is add a layer of obfuscation to what's going on. You still need to know that it returns definitions and fields, in that order, if you want to use it. So if you have to know that, why bother calling the method in the first place?

As for alternative code, I'd do things much like sschmeck already suggested, though I'd still split the task into multiple methods rather than one big one.

Answer 2

As a quick hack, with room for improvement I would suggest the following:

Definition = Struct.new(:id, :name) do
  def mapping_key(count_for_name)
    if count_for_name > 1
      "#{name}_#{id}"
    else
      name
    end
  end
end

Field = Struct.new(:id, :value)

module Items
  def items
    @items ||= {}
  end

  def add(new_item)
    items[item_key(new_item)] ||= []
    items[item_key(new_item)] << new_item
    self
  end

  def to_s
    items.to_s
  end
end

class Definitions
  include Items

  def item_key(item)
    item.name
  end

  def map_to_fields(fields)
    items.each_with_object({}) do |(_name, items), result|
      items.each do |item|
        result[item.mapping_key(items.count)] = fields.value_for(item.id)
      end
    end
  end
end

class Fields
  include Items

  def item_key(item)
    item.id
  end

  def value_for(key)
    values_for(key).first.value
  end

  def values_for(key)
    items[key]
  end
end

definitions = Definitions.new.add(Definition.new(1, 'custom def 1'))
                  .add(Definition.new(2, 'custom def 2'))
                  .add(Definition.new(3, 'custom def 2'))

fields = Fields.new.add(Field.new(1, 'Value For Custom Definition 1'))
             .add(Field.new(2, 'Value For Custom Definition 2'))
             .add(Field.new(3, 'Value For Custom Definition 3'))

puts definitions.map_to_fields(fields)

Answer 3

Just a shorter implementation using Enumerable#each_with_object and Enumerable#group_by.

def build_search_to_replace_mapping(fields, definitions)
  ds_by_name = definitions.group_by { |e| e[:name] }.values
  f_value_per_id = fields.each_with_object({}) { |e,o| o[e[:id]] = e[:value] }

  ds_by_name.each_with_object({}) do |ds,o|
    with_id = ds.size > 1
    ds.each do |d|
      name, id = d[:name], d[:id]
      key = with_id ? "{{#{name}_#{id}}}" : "{{#{name}}}"
      o[key] = f_value_per_id[id]
    end
  end
end