If I call the function anagrams
below in irb, I get a non-empty hash container as expected. But if you comment out the print "No Key\n"
line, the returned hash container is now empty. In fact for all elements in list the code in the elsif
branch seems to execute. Either I'm going nuts or there is a nasty bug here:
def anagrams(list = ['cars', 'for', 'potatoes', 'racs', 'four','scar', 'creams', 'scream'])
aHash = Hash.new()
list.each { |el|
aKey = el.downcase.chars.sort.to_a.hash
if aHash.key?(aKey)
# print "Has Key\n"
aHash[aKey] << el
elsif
# print "No Key\n"
aHash[aKey] = [el]
end
}
return aHash
end
I have the following versions of ruby and irb installed:
ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-linux]
irb 0.9.6(09/06/30)
Your problem is that you're using an elsif
where you mean else
. This:
elsif
print "No Key\n"
aHash[aKey] = [el]
is misleading formatting, it is actually interpreted more like this:
elsif(print "No Key\n")
aHash[aKey] = [el]
but print
returns nil
so the logic is like this:
elsif(nil)
aHash[aKey] = [el]
and nil
is false in a boolean context so aHash[aKey] = [el]
never occurs. If you remove the print
then you end up with this:
elsif(aHash[aKey] = [el])
and the assignment occurs; the assignment is also true in a boolean context (because an Array is) but the truthiness is irrelevant in this case.
You want to use else
here:
if aHash.key?(aKey)
aHash[aKey] << el
else
aHash[aKey] = [el]
end
Even better would be to use a Hash with an Array (via a block) as its default value:
aHash = Hash.new { |h, k| h[k] = [ ] }
and then you don't need the if
at all, you can just do this:
list.each do |el|
aKey = el.downcase.chars.sort.to_a.hash
aHash[aKey] << el
end
And you can use anything as a key in a Ruby Hash so you don't even need to .to_a.hash
, you can simply use the Array itself as the key; furthermore, sort
will give you an array so you don't even need the to_a
:
list.each { |el| aHash[el.downcase.chars.sort] << el }
Someone will probably complain about the return
at the end of your method so I'll do it: you don't need the return
at the end of your method, just say aHash
and it will be the method's return value:
def anagrams(list = ['cars', 'for', 'potatoes', 'racs', 'four','scar', 'creams', 'scream'])
aHash = Hash.new { |h, k| h[k] = [ ] }
list.each { |el| aHash[el.downcase.chars.sort] << el }
aHash
end
You could also use each_with_object
to compress it even more:
def anagrams(list = ['cars', 'for', 'potatoes', 'racs', 'four','scar', 'creams', 'scream'])
list.each_with_object(Hash.new { |h, k| h[k] = [ ] }) do |el, h|
h[el.downcase.chars.sort] << el
end
end
but I'd probably do it like this to cut down on the noise:
def anagrams(list = ['cars', 'for', 'potatoes', 'racs', 'four','scar', 'creams', 'scream'])
h = Hash.new { |h, k| h[k] = [ ] }
list.each_with_object(h) { |el, h| h[el.downcase.chars.sort] << el }
end