# Acquiring a hash containing a key and a value

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

• Are you looking for a solution with your Hashes, or a solution with classes? – slowjack2k Oct 26 '16 at 21:15
• Do you know Struct and OpenStruct? They create classes and/or objects from Hashes. – sschmeck Oct 27 '16 at 6:05
• @slowjack2k either is fine. A solution with hashes might be easier since my code does run. – Rhs Oct 27 '16 at 16:57

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


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.

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

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'))

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)


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