Reputation: 203
Below I have written a piece of code that's supposed to count the number of occurrences of characters in a string and display the result in a hash. My idea was to get rid of that letter in the string right after I've counted it so that we don't put it into the hash more than once.
My original code was this:
def letter_count_spec(str)
letter_count = Hash.new #create hash
letter = str.split('') #separate letters
letter.each{ |e|
if( /[A-Za-z0-9]/.match(e) )
occurances = letter.count{ |x| x==e}
letter_count[e] = occurances
letter.delete(e) #delete letter
end
}
return letter_count
end
letter_count_spec("cat")
result: => {"c"=>1, "t"=>1}
I lose "a"!
So I tried this:
def letter_count_spec(str)
letter_count = Hash.new #create hash
letter = str.split('') #separate letters
letter.each{ |e|
if( /[A-Za-z0-9]/.match(e) )
occurances = letter.count{ |x| x==e}
letter_count[e] = occurances
end
}
letter.each{ |e|
letter.delete(e) #delete letter
}
return letter_count
end
letter_count_spec("cat")
result => {"a"=>1, "c"=>1, "t"=>1}
Why would I need to go through the array again to delete?
Upvotes: 0
Views: 65
Reputation: 14082
The modification of collection during iteration may cause problems, which is stated in the comment.
The word counting algorithm normally involves a hash to keep track of the word count, and an iterator to go through the content. You don't need to modify the original collection. This is an O(n) solution, given that the hash has O(1) complexity on update in general case. However, the count and delete approach in your post has O(n^2) complexity (if it works).
def letter_count_spec(str)
letter_count = Hash.new(0) # create hash, and use 0 as the default value
letter = str.split('') # separate letters
letter.each do |e|
if /[A-Za-z0-9]/.match(e)
letter_count[e] += 1 # increment count
end
end
letter_count
end
BTW, in Ruby, it's a convention to use do ... end
for multiple lines block, unless {}
is required in some circumstances.
Upvotes: 1